Re: [PATCH v4 4/9] perf c2c: add HPP list parsing for function view columns

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims to modify the core `perf_hpp__setup_output_field()` function to operate on a caller-provided list, but the patch actually implements a local workaround function instead.
--

--- Patch [4]: [PATCH v4 4/9] perf c2c: add HPP list parsing for function view columns ---
commit c0d30306be98707f48f543440ba43c56408600b2
Author: Jiebin Sun <jiebin.sun@intel.com>

perf c2c: add HPP list parsing for function view columns

This commit adds parsing to turn an output/sort column string into the function
view's hpp_list. It includes dimension lookup, comparator wrappers, format
allocation, and initialization entry points.

> Make perf_hpp__setup_output_field() register appended sort keys on the
> hpp_list passed by its caller.  It previously used the global-list wrapper,
> which left a local list without output fields and linked its formats into
> the global perf_hpp_list instead.

[Severity: Medium]
This isn't a bug, but the commit message states that it modifies the core
perf_hpp__setup_output_field() function to operate on a caller-provided list.
However, the patch does not actually modify perf_hpp__setup_output_field().

> diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> index 4eda97894d1d4..21e156bbbef35 100644
> --- a/tools/perf/ui/browsers/c2c-function.c
> +++ b/tools/perf/ui/browsers/c2c-function.c
[ ... ]
> @@ -400,13 +400,277 @@ static struct c2c_dimension dim_symbol_view = {
[ ... ]
> +/*
> + * Append the function view's sort keys to its own output fields, mirroring
> + * perf_hpp__setup_output_field() but on the local @list. The shared helper
> + * registers onto the global perf_hpp_list, which would leave this local list
> + * without output columns, so the function view keeps its own copy here.
> + */
> +static void c2c_function_hists__setup_output_field(struct perf_hpp_list *list)
> +{
> +	struct perf_hpp_fmt *fmt;
> +

Could the commit message be updated to accurately reflect that this local
helper c2c_function_hists__setup_output_field() was introduced rather than
modifying the core function?

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