lkml.org 
[lkml]   [2014]   [Nov]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH 04/12 v3] tracing: Convert seq_buf_path() to be like seq_path()
    On Wed, 5 Nov 2014 15:45:53 +0100
    Petr Mladek <pmladek@suse.cz> wrote:


    > > */
    > > -int seq_buf_path(struct seq_buf *s, const struct path *path)
    > > +int seq_buf_path(struct seq_buf *s, const struct path *path, const char *esc)
    > > {
    > > - unsigned int len = SEQ_BUF_LEFT(s);
    > > - unsigned char *p;
    > > -
    > > - WARN_ON(s->size == 0);
    >
    > I would keep this check.

    Yeah, I could.

    >
    > > - p = d_path(path, s->buffer + s->len, len);
    > > - if (!IS_ERR(p)) {
    > > - p = mangle_path(s->buffer + s->len, p, "\n");
    > > - if (p) {
    > > - s->len = p - s->buffer;
    > > - return 0;
    > > + char *buf = s->buffer + s->len;
    > > + size_t size = SEQ_BUF_LEFT(s);
    >
    > I would use the variable name "len" to make it consistent with
    > the other fucntions in seq_buf.c.

    Note, seq_path() is a different beast than the other seq_*() functions
    (this will be keeping a return code). And the inconsistency is in
    seq_file.c as well. I'm not saying we shouldn't keep it consistent. But
    as this patch is to make seq_buf like seq_file, I'll keep the
    inconsistencies the same too.

    We can always do a clean up later.


    >
    > > + int res = -1;
    > > +
    > > + if (size) {
    > > + char *p = d_path(path, buf, size);
    > > + if (!IS_ERR(p)) {
    > > + char *end = mangle_path(buf, p, esc);
    > > + if (end)
    > > + res = end - buf;
    > > }
    > > }
    > > - seq_buf_set_overflow(s);
    >
    > We still should set overflow on failure.

    Again, this is different in seq_file too. I'm leaving it as is.

    >
    > > - return -1;
    > > + if (res > 0)
    > > + s->len += res;
    > > +
    > > + return res;
    >
    > It returns -1 on failure and the number of written characters on
    > success. This is incompatible with the other seq_buf functions
    > and with the comment above this function. Also it changes the
    > return value from trace_seq_path().
    >
    > I do not mind about the used scheme but I think that we should
    > make it consistent.
    >

    As seq_file has had this inconsistency for a long time, and this code
    is to try to merge the code between trace_seq and seq_file, I'm going
    to follow seq_file as that has been around much longer than trace_seq.

    -- Steve



    \
     
     \ /
      Last update: 2014-11-05 21:41    [W:5.781 / U:0.136 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site