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:
- [Medium] Silent error masking and inconsistent propagation for -ENOMEM during sort and output token parsing in setup_sort_list() and setup_output_list().
--

--- Patch [1]: [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() ---
commit c1a9e4a1694a477755b6abff539aa40b1fed31e1
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 three bugs in hpp_list__parse() by checking strdup()
return values and adding a cleanup label to preserve early errors. It also
converts PARSE_LIST to a function, changing called functions to return
appropriate error codes like -ENOMEM instead of -1 on failure.

> 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

[ ... ]

> @@ -3869,8 +3877,7 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,
>  				    strlen(tok)))
>  			return -EINVAL;
>  
> -		__sort_dimension__add(sd, list, level);
> -		return 0;
> +		return __sort_dimension__add(sd, list, level);
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(memory_sort_dimensions); i++) {

[Severity: Medium]
Since sort_dimension__add() can now return -ENOMEM, could this introduce a
regression where the callers in setup_sort_list() and setup_output_list()
fail to handle the new return value?

Currently, setup_sort_list() only checks for -EINVAL and -ESRCH:

tools/perf/util/sort.c:setup_sort_list() {
    ...
		ret = sort_dimension__add(list, tok, evlist, env, level);
		if (ret == -EINVAL) {
			...
			break;
		} else if (ret == -ESRCH) {
			...
			break;
		}
    ...
}

If sort_dimension__add() returns -ENOMEM, the loop will not break and ret
will be silently overwritten on the next iteration.

A similar issue appears to exist in setup_output_list() which ignores
-ENOMEM from output_field_add().

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