Linux Perf Users
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Arnaldo Carvalho de Melo" <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free()
Date: Mon, 15 Jun 2026 23:04:13 +0000	[thread overview]
Message-ID: <20260615230414.0914D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260615223249.36598-8-acme@kernel.org>

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

Pre-existing issues:
- [Medium] perf_hpp__setup_output_field() and perf_hpp__append_sort_keys() append formats to the global perf_hpp_list instead of the provided local list, polluting global state and causing missing output columns.
--

commit a4f0bc9a81bce084555c8dfe98ae2f6675da0ef8
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf c2c: Fix hist entry and format list leaks in c2c_he_free()

This commit addresses leaks in c2c_he_free() by replacing
hists__delete_entries() with hists__delete_all_entries() to walk and free
all inner hist_entry objects across all rb_root trees. It also adds a call
to perf_hpp__reset_output_field() to unregister and free dynamically
allocated format entries on the hists->list.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index eabb922ef295e..c9584dbedf77a 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -184,7 +184,7 @@ static void c2c_he_free(void *he)
>  
>  	c2c_he = container_of(he, struct c2c_hist_entry, he);
>  	if (c2c_he->hists) {
> -		hists__delete_entries(&c2c_he->hists->hists);
> +		hists__delete_all_entries(&c2c_he->hists->hists);
>  		perf_hpp__reset_output_field(&c2c_he->hists->list);

[Severity: Medium]
This isn't a bug introduced by this patch, but does the setup for these local
hpp lists append formats to the global perf_hpp_list by mistake?

Looking at perf_hpp__setup_output_field() in tools/perf/ui/hist.c, it takes a
local list argument but uses the global wrapper perf_hpp__column_register(fmt)
instead of perf_hpp_list__column_register(list, fmt).

A similar issue appears in perf_hpp__append_sort_keys() in the same file,
which uses perf_hpp__register_sort_field(fmt) instead of
perf_hpp_list__register_sort_field(list, fmt).

Does this inject sort formats into the global list (polluting global state
during processing) while leaving the local list missing those columns and
breaking the output display for inner hists?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615223249.36598-1-acme@kernel.org?part=7

  reply	other threads:[~2026-06-15 23:04 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 1/9] perf machine: Propagate machine__init() error to callers Arnaldo Carvalho de Melo
2026-06-15 22:52   ` sashiko-bot
2026-06-15 22:32 ` [PATCH 2/9] perf machine: Use snprintf() for guestmount path construction Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 3/9] perf cs-etm: Validate num_cpu before metadata allocation Arnaldo Carvalho de Melo
2026-06-15 22:48   ` sashiko-bot
2026-06-15 22:32 ` [PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check Arnaldo Carvalho de Melo
2026-06-15 22:46   ` sashiko-bot
2026-06-15 22:32 ` [PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue() Arnaldo Carvalho de Melo
2026-06-15 22:54   ` sashiko-bot
2026-06-15 22:32 ` [PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free() Arnaldo Carvalho de Melo
2026-06-15 23:04   ` sashiko-bot [this message]
2026-06-15 22:32 ` [PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers Arnaldo Carvalho de Melo
2026-06-15 23:01   ` sashiko-bot
2026-06-15 22:32 ` [PATCH 9/9] perf dso: Set standard errno on decompression failure Arnaldo Carvalho de Melo
  -- strict thread matches above, loose matches on Subject: below --
2026-06-16  2:27 [PATCHES v4 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
2026-06-16  2:27 ` [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free() Arnaldo Carvalho de Melo
2026-06-16  1:08 [PATCHES v3 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
2026-06-16  1:08 ` [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free() Arnaldo Carvalho de Melo
2026-06-15 21:36 [PATCHES v1 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
2026-06-15 21:36 ` [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free() Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260615230414.0914D1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=acme@kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox