Re: [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The string parsing loops in `setup_sort_list()` and `setup_output_list()` silently ignore and overwrite memory allocation errors (-ENOMEM), leading to partially initialized sort or output fields being treated as successful.
--

--- Patch [1]: [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() ---
commit eb21379d23b3ae985cd662b484d5da281ac1387b
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse()

This commit fixes multiple issues in hpp_list__parse(), including error
masking during string parsing, unhandled strdup() OOM failures, and unchecked
function returns. It also updates underlying sort and output functions to
return -ENOMEM instead of -1 on allocation failures.

> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 005e7d85dc4a5..616a583e3d4d6 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
[ ... ]
> @@ -3882,8 +3889,7 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,
>  		if (sort__mode != SORT_MODE__MEMORY)
>  			return -EINVAL;
>  
> -		__sort_dimension__add(sd, list, level);
> -		return 0;
> +		return __sort_dimension__add(sd, list, level);
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(hpp_sort_dimensions); i++) {

[Severity: Medium]
This is a pre-existing issue, but by explicitly propagating -ENOMEM out of
sort_dimension__add() and output_field_add(), does this highlight a bug in
their callers?

In setup_sort_list() and setup_output_list(), the parsing loops only check
for -EINVAL and -ESRCH:

ret = sort_dimension__add(list, tok, evlist, env, level);
if (ret == -EINVAL) {
	...
	break;
} else if (ret == -ESRCH) {
	...
	break;
}

If sort_dimension__add() or output_field_add() returns -ENOMEM, the loop
continues without breaking, and a subsequent successful token will overwrite
the error. This can cause the perf tool to silently continue with a partially
configured state instead of aborting on memory allocation failure.

Should these loops be updated to catch other error codes like -ENOMEM?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180703.194916-1-acme@kernel.org?part=1