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

[Ian Rogers]

On Wed, Aug 5, 2026 at 8:11 AM Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
>
> When c2c_hists__init() or c2c_hists__reinit() calls hpp_list__parse()
> and it fails partway through, format structures registered via
> perf_hpp_list__column_register() and perf_hpp_list__register_sort_field()
> are left on the hpp_list.
>
> In c2c_hists__init(), only one of the callers, c2c_he__alloc_hists(),
> handled this with perf_hpp__reset_output_field(), while perf_c2c_report()
> did not, leaking the partially registered entries.
>
> In c2c_hists__reinit(), neither perf_c2c_report() nor resort_cl_cb()
> clean up on failure.
>
> Fix by adding cleanup inside both functions themselves, so all callers
> are protected, and remove the now redundant reset in c2c_he__alloc_hists().
>
> Fixes: 78b275437873 ("perf c2c report: Add sample processing")
> Reported-by: sashiko-bot <sashiko-bot@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Assisted-by: Claude:claude-opus-4.6
> Assisted-by: Opencode:mimo-v2.5-free
> Assisted-by: Opencode:DeepSeek-V4-Flash-free
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
>  tools/perf/builtin-c2c.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 160b82694d391c50..bc16a57e092742ad 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -229,7 +229,6 @@ he__get_c2c_hists(struct hist_entry *he,
>
>         ret = c2c_hists__init(hists, sort, nr_header_lines, env);
>         if (ret) {
> -               perf_hpp__reset_output_field(&hists->list);
>                 c2c_he->hists = NULL;
>                 free(hists);
>                 return NULL;
> @@ -2147,6 +2146,8 @@ static int c2c_hists__init(struct c2c_hists *hists,
>                            int nr_header_lines,
>                            struct perf_env *env)
>  {
> +       int ret;
> +
>         __hists__init(&hists->hists, &hists->list);
>
>         /*
> @@ -2159,7 +2160,13 @@ static int c2c_hists__init(struct c2c_hists *hists,
>         /* Overload number of header lines.*/
>         hists->list.nr_header_lines = nr_header_lines;
>
> -       return hpp_list__parse(&hists->list, /*output=*/NULL, sort, env);
> +       ret = hpp_list__parse(&hists->list, /*output=*/NULL, sort, env);
> +
> +       /* Unregister any formats added before the failure point */
> +       if (ret)
> +               perf_hpp__reset_output_field(&hists->list);
> +
> +       return ret;
>  }
>
>  static int c2c_hists__reinit(struct c2c_hists *c2c_hists,
> @@ -2167,8 +2174,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 = 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;
>  }
>
>  #define DISPLAY_LINE_LIMIT  0.001
> --
> 2.55.0
>