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

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

Pre-existing issues:
- [Medium] print_pareto() calls hpp_list__parse() on a local stack variable=
 but never cleans it up, leaking dynamically allocated formats.
- [Medium] kvm_hists__init() and kvm_hists__reinit() suffer from identical =
partial-registration leaks when format parsing fails.
--

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

perf c2c: Clean up registered formats on c2c_hists__init() and c2c_hists__r=
einit() failure

When c2c_hists__init() or c2c_hists__reinit() calls hpp_list__parse()
and it fails partway through, format structures registered are left on
the hpp_list. This commit adds cleanup inside both functions to protect
all callers from leaking partially registered entries.

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

[ ... ]

> @@ -2167,8 +2175,16 @@ static int c2c_hists__reinit(struct c2c_hists *c2c=
_hists,
>  			     const char *sort,
>  			     struct perf_env *env)
>  {
> +	int ret;
> +
>  	perf_hpp__reset_output_field(&c2c_hists->list);
> -	return hpp_list__parse(&c2c_hists->list, output, sort, env);
> +	ret =3D hpp_list__parse(&c2c_hists->list, output, sort, env);
> +
> +	/* Unregister any formats added before the failure point */
> +	if (ret)
> +		perf_hpp__reset_output_field(&c2c_hists->list);
> +
> +	return ret;
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but I noticed that print_pareto()
in the same file calls hpp_list__parse() on a local stack variable:

tools/perf/builtin-c2c.c:print_pareto() {
    ...
    ret =3D hpp_list__parse(&hpp_list, cl_output, /*evlist=3D*/NULL, env);
    ...
}

Because print_pareto() returns without ever calling=20
perf_hpp__reset_output_field(), does it leak all allocated c2c_fmt structur=
es=20
on both the success and failure paths?

[Severity: Medium]
This is a pre-existing issue, but kvm_hists__init() and kvm_hists__reinit()=
 in
tools/perf/builtin-kvm.c seem to suffer from identical partial-registration
leaks when format parsing fails:

tools/perf/builtin-kvm.c:kvm_hists__init() {
    ...
    return kvm_hpp_list__parse(&kvm_hists.list, NULL, "ev_name");
}

tools/perf/builtin-kvm.c:kvm_hists__reinit() {
    ...
    return kvm_hpp_list__parse(&kvm_hists.list, output, sort);
}

If parsing fails partway through these functions, will partially allocated
formats remain in the global list without cleanup, replicating the exact le=
ak
pattern fixed here in c2c?

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