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.
> + 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:
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.
-- 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;
> +}
> +
