On Fri, 17 Apr 2026 20:24:00 +0800
Pengpeng Hou <[email protected]> wrote:
> diff --git a/kernel/trace/trace_events_hist.c
> b/kernel/trace/trace_events_hist.c
> index 73ea180cad55..954e0beb7f0a 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -1764,13 +1764,14 @@ static void expr_field_str(struct hist_field *field,
> char *expr)
> static char *expr_str(struct hist_field *field, unsigned int level)
> {
> char *expr;
> + int ret = -EINVAL;
No need for the ret value, use:
char *expr __free(kfree) = NULL;
instead.
>
> if (level > 1)
> - return NULL;
> + return ERR_PTR(-EINVAL);
>
> expr = kzalloc(MAX_FILTER_STR_VAL, GFP_KERNEL);
> if (!expr)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
>
> if (!field->operands[0]) {
> expr_field_str(field, expr);
> @@ -1782,9 +1783,9 @@ static char *expr_str(struct hist_field *field,
> unsigned int level)
>
> strcat(expr, "-(");
> subexpr = expr_str(field->operands[0], ++level);
> - if (!subexpr) {
> - kfree(expr);
> - return NULL;
> + if (IS_ERR(subexpr)) {
> + ret = PTR_ERR(subexpr);
> + goto free;
The above could then be:
if (IS_ERR(subexpr))
return subexpr;
> }
> strcat(expr, subexpr);
> strcat(expr, ")");
> @@ -1810,13 +1811,16 @@ static char *expr_str(struct hist_field *field,
> unsigned int level)
> strcat(expr, "*");
> break;
> default:
> - kfree(expr);
> - return NULL;
This could be just:
return ERR_PTR(-EINVAL);
> + goto free;
> }
>
> expr_field_str(field->operands[1], expr);
>
> return expr;
And the above would be:
return_ptr(expr);
> +
> +free:
> + kfree(expr);
> + return ERR_PTR(ret);
And the above isn't needed.
-- Steve
> }
>
