Re: [PATCH 2/2] perf c2c: Clean up registered formats on c2c_hists__init() failure

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

Pre-existing issues:
- [High] perf_hpp__setup_output_field() and perf_hpp__append_sort_keys() co=
rrupt the global perf_hpp_list and fail to populate the provided local list.
--

--- Patch [2]: [PATCH 2/2] perf c2c: Clean up registered formats on c2c_his=
ts__init() failure ---
commit 0cc1bae097f820bedd8cd5517b56916ee9c48ca3
Author: Arnaldo Carvalho de Melo <[email protected]>

perf c2c: Clean up registered formats on c2c_hists__init() failure

When c2c_hists__init() calls hpp_list__parse() and it fails partway through,
format structures registered are left on the hpp_list. This fix adds cleanup
inside c2c_hists__init() itself so all callers are protected.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 160b82694d391..a01fa1655569c 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c

[ ... ]

> @@ -2159,7 +2161,13 @@ static int c2c_hists__init(struct c2c_hists *hists,
>  	/* Overload number of header lines.*/
>  	hists->list.nr_header_lines =3D nr_header_lines;
> =20
> -	return hpp_list__parse(&hists->list, /*output=3D*/NULL, sort, env);
> +	ret =3D hpp_list__parse(&hists->list, /*output=3D*/NULL, sort, env);

[Severity: High]
This isn't a bug introduced by this patch, but will calling hpp_list__parse=
()
corrupt the global perf_hpp_list?

When c2c initializes its local hpp_list, hpp_list__parse() eventually leads
to calls to perf_hpp__setup_output_field() and perf_hpp__append_sort_keys()
in tools/perf/ui/hist.c. Both of these functions accept a local
struct perf_hpp_list *list parameter, but they appear to use the global list
wrappers instead of modifying the passed local list:

tools/perf/ui/hist.c:perf_hpp__setup_output_field() {
    ...
    perf_hpp__column_register(fmt);
    ...
}

tools/perf/ui/hist.c:perf_hpp__append_sort_keys() {
    ...
    perf_hpp__register_sort_field(fmt);
    ...
}

Could this cause the local lists to miss output columns, while also polluti=
ng
the global list with dangling pointers to dynamically allocated local forma=
ts?

Should these functions be updated to call
perf_hpp_list__column_register(list, fmt) and
perf_hpp_list__register_sort_field(list, fmt) instead?

> +
> +	/* Unregister any formats added before the failure point */
> +	if (ret)
> +		perf_hpp__reset_output_field(&hists->list);
> +
> +	return ret;
>  }
>

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803180703.1949=
[email protected]?part=3D2