On Thu, 15 Oct 2020 09:09:55 -0400 Steven Rostedt <[email protected]> wrote:
> On Thu, 15 Oct 2020 18:00:08 +0900 > Masami Hiramatsu <[email protected]> wrote: > > > +#define STATIC_FMT_BUF_SIZE 128 > > +static char static_fmt_buf[STATIC_FMT_BUF_SIZE]; > > + > > +const char *trace_event_format(struct trace_iterator *iter, const char > > *fmt) > > +{ > > + const char *p, *new_fmt; > > + char *q; > > + > > + if (WARN_ON_ONCE(!fmt)) > > + return fmt; > > +retry: > > + p = fmt; > > + new_fmt = q = iter->fmt; > > + while (*p) { > > + if (unlikely(q - new_fmt + 3 > iter->fmt_size)) { > > + /* expand format buffer if needed */ > > + if (iter->fmt == static_fmt_buf) > > + return fmt; > > + > > + iter->fmt_size = iter->fmt_size ? iter->fmt_size * 2 > > + : STATIC_FMT_BUF_SIZE; > > Doubling may be too much, as fmt strings are not going to grow > exponentially. OK, then I just make it "+= STATIC_FMT_BUF_SIZE". Actually, as far as I can see, the format string should be shorter than 128 bytes. If we see it is longer than that, it will be 2 lines in event definition. And 128 + 128 = 256, which will be longer than 3 lines in 80 columns wide terminal. I guess no one want to write it. :) > > > + kfree(iter->fmt); > > + iter->fmt = kmalloc(iter->fmt_size, GFP_KERNEL); > > Need to test the return value of kmalloc. But I would use realloc instead, > and continue: Good catch! > > char *tmp_fmt; > unsigned int tmp_size; > > tmp_size = iter->fmt_size + STATIC_FMT_BUF_SIZE; > > tmp_fmt = krealloc(iter->fmt, tmp_size, GFP_KERNEL); > if (!tmp_fmt) > return fmt; > > q += tmp_fmt - new_fmt; > new_fmt = iter->fmt = tmp_fmt; > iter->fmt_size = tmp_size; > > > + goto retry; > > Then you don't even need the retry, and just continue processing. > > Also, if it fails to allocate, you still have the iter->fmt and fmt_size > intact and correct. OK, let me update it. Thank you, > > -- Steve > > > + } > > + *q++ = *p++; > > + /* Replace %p with %px */ > > + if (p[-1] == '%') { > > + if (p[0] == '%') { > > + *q++ = *p++; > > + } else if (p[0] == 'p' && !isalnum(p[1])) { > > + *q++ = *p++; > > + *q++ = 'x'; > > + } > > + } > > + } > > + *q = '\0'; > > + > > + return new_fmt; > > +} > > + -- Masami Hiramatsu <[email protected]>
