On Fri, Jan 16, 2015 at 02:22:02AM +0000, Steven Rostedt wrote:
> On Thu, 15 Jan 2015 16:50:58 +0000
> Javi Merino <javi.mer...@arm.com> wrote:
>
> > +const char *
> > +ftrace_print_array_seq(struct trace_seq *p, const void *buf, int
> > buf_len,
> > + size_t el_size)
> > +{
> > + const char *ret = trace_seq_buffer_ptr(p);
> > + const char *prefix = "";
> > + void *ptr = (void *)buf;
> > +
> > + trace_seq_putc(p, '{');
> > +
> > + while (ptr < buf + buf_len) {
> > + switch (el_size) {
> > + case 8:
> > + trace_seq_printf(p, "%s0x%x", prefix,
> > + *(u8 *)ptr);
> > + break;
> > + case 16:
> > + trace_seq_printf(p, "%s0x%x", prefix,
> > + *(u16 *)ptr);
> > + break;
> > + case 32:
> > + trace_seq_printf(p, "%s0x%x", prefix,
> > + *(u32 *)ptr);
> > + break;
> > + case 64:
> > + trace_seq_printf(p, "%s0x%llx", prefix,
> > + *(u64 *)ptr);
> > + break;
> > + default:
> > + BUG();
>
> BUG() is a bit extreme don't you think? I'm not sure it even deserves a
> WARN_ON().
Ok, I used BUG() because that's what you suggested:
http://article.gmane.org/gmane.linux.kernel/1846749
The only way I could think of turning it into a BUILD_BUG was by
moving it to the __print_array macro, but I think it's ugly.
> I would suggest doing:
>
> trace_seq_printf(p, "BAD SIZE:%d 0x%x", el_size,
> *(u8 *)ptr);
> el_size = 8;
>
> No need to go crashing the kernel or even messing with dmesg over
> somebody's tracepoint mistake.
Ok, I'll change it to that.
> The rest looks fine.
>
> > + }
> > + prefix = ",";
> > + ptr += el_size / 8;
> > + }
> > +
> > + trace_seq_putc(p, '}');
> > + trace_seq_putc(p, 0);
>
> I need to add a trace_seq_terminate() for this.
That would make it more readable. Cheers,
Javi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
