Re: [PATCH v3 1/2] tracing: Return ERR_PTR() from expr_str()

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 17 Apr 2026 20:24:00 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> 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

>  }
>