Re: [PATCH 08/14] perf c2c: add HPP list parsing for function view histograms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] OOM failures in get_function_format() are swallowed and misreported as invalid dimensions.
--

commit c2feb45b926c3d4847f0ee3b178a0bfe059a7079
Author: Jiebin Sun <jiebin.sun@intel.com>

perf c2c: add HPP list parsing for function view histograms

This commit adds the histogram initialization and column parsing
infrastructure for the perf c2c function view. It bridges custom dimensions
with the perf HPP list infrastructure to enable dynamic column configuration.

> diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> index 794a023239ce1..c11a81d93c21e 100644
> --- a/tools/perf/ui/browsers/c2c-function.c
> +++ b/tools/perf/ui/browsers/c2c-function.c
[ ... ]
> @@ -509,6 +509,146 @@ static __maybe_unused struct c2c_fmt *get_function_format(const char *name)
>  	return c2c_fmt;
>  }
>  
> +static int
> +c2c_function_hists__init_output(struct perf_hpp_list *hpp_list, char *name,
> +				struct perf_env *env __maybe_unused)
> +{
> +	struct c2c_fmt *c2c_fmt = get_function_format(name);
> +	int level = 0;
> +
> +	if (!c2c_fmt) {
> +		reset_dimensions();
> +		return output_field_add(hpp_list, name, &level);
> +	}

[Severity: Medium]
If zalloc() fails inside get_function_format(), it returns NULL. Does this
fallback path swallow the -ENOMEM and mistakenly treat it as an unknown
dimension?

Since it falls back to output_field_add() which will fail with -EINVAL, the
caller function_hpp_list__add_tokens() might print the misleading error
'Invalid c2c function-view field' instead of reporting the memory allocation
failure.

[ ... ]
> +static int
> +c2c_function_hists__init_sort(struct perf_hpp_list *hpp_list, char *name,
> +			      struct perf_env *env)
> +{
> +	struct c2c_fmt *c2c_fmt = get_function_format(name);
> +
> +	if (!c2c_fmt) {
> +		reset_dimensions();
> +		return sort_dimension__add(hpp_list, name, /*evlist=*/NULL, env, /*level=*/0);
> +	}

[Severity: Medium]
Similarly here, does this path also swallow an -ENOMEM from
get_function_format() and mistakenly treat it as an unknown dimension?

> +
> +	/* Mark symbol-backed sort keys so hists__has(hists, sym) is correct. */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626070355.1556721-1-jiebin.sun@intel.com?part=8