Messages in this thread | | | Date | Wed, 5 Nov 2014 15:00:07 -0500 | From | Steven Rostedt <> | Subject | Re: [RFC][PATCH 03/12 v3] tracing: Create seq_buf layer in trace_seq |
| |
On Wed, 5 Nov 2014 13:41:47 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> > > > > + */ > > > +int seq_buf_bitmask(struct seq_buf *s, const unsigned long *maskp, > > > + int nmaskbits) > > > +{ > > > + unsigned int len = SEQ_BUF_LEFT(s); > > > > > > + int ret; > > > + > > > + WARN_ON(s->size == 0); > > > + > > > + if (s->len < s->size) { > > > + ret = bitmap_scnprintf(s->buffer, len, maskp, nmaskbits); > > > > It writes to the beginning of the buffer. It should be > > > > ret = bitmap_scnprintf(s->buffer + s->len, len, > > maskp, nmaskbits); > > > > Yep thanks. Luckily its only user didn't care. > > Will fix. > > > > > > + if (s->len + ret < s->size) { > > > > This will always happen because bitmap_scnprintf() is limited by SEQ_BUF_LEFT(s) > > and it currently returns the remaining size - len - 1. > > Hmm, that's correct, as bitmap_scnprintf() returns the amount written > instead of the amount that it would write like snprintf() would. > > > > > > You might want to use "s->size - s->len" instead of SEQ_BUF_LEFT(s). > > That wont help when we make overflow len > size. > > Probably should see if ret == the amount of bits required for the > bitmask.
Here's the new version:
int seq_buf_bitmask(struct seq_buf *s, const unsigned long *maskp, int nmaskbits) { unsigned int len = seq_buf_buffer_left(s); int size; int ret;
WARN_ON(s->size == 0);
if (s->len < s->size) { /* * Calculate to see if we have enough room to fit * @nmaskbits as a string */ size = (nmaskbits + 3) / 4; /* Add the commas that are used for groups of 8 hex digits */ size += size / 8; if (len >= size) { ret = bitmap_scnprintf(s->buffer, len, maskp, nmaskbits); WARN_ON_ONCE(s->len + ret >= s->size); s->len += ret; return 0; } } seq_buf_set_overflow(s); return -1; }
> > > > > > > > + s->len += ret; > > > + return 0; > > > + } > > > + } > > > + seq_buf_set_overflow(s); > > > + return -1; > > > +}
> > > +/** > > > + * seq_buf_putmem_hex - write raw memory into the buffer in ASCII hex > > > + * @s: seq_buf descriptor > > > + * @mem: The raw memory to write its hex ASCII representation of > > > + * @len: The length of the raw memory to copy (in bytes) > > > + * > > > + * This is similar to seq_buf_putmem() except instead of just copying the > > > + * raw memory into the buffer it writes its ASCII representation of it > > > + * in hex characters. > > > + * > > > + * Returns zero on success, -1 on overflow > > > + */ > > > +int seq_buf_putmem_hex(struct seq_buf *s, const void *mem, > > > + unsigned int len) > > > +{ > > > + unsigned char hex[HEX_CHARS]; > > > + const unsigned char *data = mem; > > > + unsigned int start_len; > > > + int i, j; > > > + > > > + WARN_ON(s->size == 0); > > > + > > > + while (len) { > > > + start_len = min(len, HEX_CHARS - 1); > > > +#ifdef __BIG_ENDIAN > > > + for (i = 0, j = 0; i < start_len; i++) { > > > +#else > > > + for (i = start_len-1, j = 0; i >= 0; i--) { > > > +#endif > > > + hex[j++] = hex_asc_hi(data[i]); > > > + hex[j++] = hex_asc_lo(data[i]); > > > + } > > > + if (WARN_ON_ONCE(j == 0 || j/2 > len)) > > > + break; > > > + > > > + /* j increments twice per loop */ > > > + len -= j / 2; > > > + hex[j++] = ' '; > > > + > > > + seq_buf_putmem(s, hex, j); > > > + if (seq_buf_has_overflowed(s)) > > > > We might want to use the seq_buf_putmem() return value here. > > We could do that.
Actually, no we can't. As I stated before, the return values of most of the seq_*() functions (for seq_file and seq_buf) will be turning into void functions.
I'm avoiding checking return values here.
-- Steve
> > > > > > + return -1; > > > + } > > > + return 0; > > > +} > > > + > >
| |