Re: [PATCH v4 2/2] tracing: Bound histogram expression strings with seq_buf

[Steven Rostedt]

On Thu, 21 May 2026 10:28:17 +0800
Pengpeng Hou <pengp...@iscas.ac.cn> wrote:

> @@ -1778,47 +1783,56 @@ static char *expr_str(struct hist_field *field, 
> unsigned int level)
>       if (!expr)
>               return ERR_PTR(-ENOMEM);
>  
> +     seq_buf_init(&s, expr, MAX_FILTER_STR_VAL);
> +
>       if (!field->operands[0]) {
> -             expr_field_str(field, expr);
> +             if (!expr_field_str(field, &s))
> +                     return ERR_PTR(-E2BIG);
> +
>               return_ptr(expr);
>       }
>  
>       if (field->operator == FIELD_OP_UNARY_MINUS) {
> -             char *subexpr;
> +             char *subexpr __free(kfree) = NULL;
>  
> -             strcat(expr, "-(");
> +             seq_buf_puts(&s, "-(");
>               subexpr = expr_str(field->operands[0], ++level);
>               if (IS_ERR(subexpr))
>                       return subexpr;
>  
> -             strcat(expr, subexpr);
> -             strcat(expr, ")");
> +             seq_buf_puts(&s, subexpr);
> +             seq_buf_putc(&s, ')');
> +             seq_buf_str(&s);
>  
> -             kfree(subexpr);
> +             if (seq_buf_has_overflowed(&s))
> +                     return ERR_PTR(-E2BIG);
>  
>               return_ptr(expr);
>       }

Wouldn't the above if statement be a lot nicer as:

        if (field->operator == FIELD_OP_UNARY_MINUS) {
                char *subexpr;

                subexpr = expr_str(field->operands[0], ++level);
                if (IS_ERR(subexpr))
                        return subexpr;

                seq_buf_printf(&s, "-(%s)", subexpr); 
                seq_buf_str(&s);
                kfree(subexpr);

                if (seq_buf_has_overflowed(&s))
                        return ERR_PTR(-E2BIG);
 
                return_ptr(expr);
        }

In fact, the above is so simple, you don't even need to use the __free()
guard on subexpr.

BTW, because currently seq_buf_printf() does add a '\0' to the string, I
may update the API for seq_buf to state that you don't need to terminate
after calling that function. So you can leave out the sub_buf_str() after
calling seq_buf_printf().

-- Steve