Re: [PATCH v6 4/6] perf tools: Show memory region in perf-c2c subcommand

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Memory leak of `output_str` on error paths in `perf_c2c__report`.
--

commit 667423b8d109e131918e4b4cfbcb77053abced1c
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
Subject: perf tools: Show memory region in perf-c2c subcommand

This patch adds a memory region field to the `perf c2c` cacheline list
view to help identify the memory region to which the cacheline belongs.
It modifies `output_str` to be dynamically allocated via `asprintf()`
when formatting the output string.

Link: https://lore.kernel.org/all/20260114011750.350569-1-dapeng1.mi@linux.intel.com/

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 715b75d42f2a8..c877b5056da08 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c

[ ... ]

> @@ -3271,9 +3325,16 @@ static int perf_c2c__report(int argc, const char **argv)
>  		goto out_mem2node;
>  	}
>  
> -	if (c2c.display != DISPLAY_SNP_PEER)
> -		output_str = "cl_idx,"
> +	c2c.show_mem_region = perf_header__has_feat(&session->header,
> +						HEADER_MEMORY_RANGES);
> +	if (c2c.show_mem_region)
> +		dim_dcacheline.header.line[0].span = 3;
> +
> +	if (c2c.display != DISPLAY_SNP_PEER) {
> +		if (asprintf(&output_str,
>  			     "cl_idx,"

[ ... ]

> @@ -3329,11 +3403,13 @@ static int perf_c2c__report(int argc, const char **argv)
>  
>  	if (ui_quirks()) {
>  		pr_err("failed to setup UI\n");
> -		goto out_mem2node;
> +		goto out_str;
>  	}
>  
>  	perf_c2c_display(session);
>  
> +out_str:
> +	free(output_str);

[Severity: Medium]
Does this patch introduce a memory leak for output_str on other error paths?

This patch changes output_str to be dynamically allocated via asprintf(),
and updates the error path for ui_quirks() to jump to out_str to free it. 

However, looking at the pre-existing code between the asprintf() calls and
ui_quirks(), there are other error paths that still jump to out_mem2node:

	err = c2c_hists__reinit(&c2c.hists, output_str, sort_str,
	                        perf_session__env(session));
	if (err) {
		pr_err("Failed to reinitialize hists\n");
		goto out_mem2node;
	}

	ui_progress__init(&prog, c2c.hists.hists.nr_entries, "Sorting...");

	hists__collapse_resort(&c2c.hists.hists, NULL);
	hists__output_resort_cb(&c2c.hists.hists, &prog, resort_shared_cl_cb);
	err = hists__iterate_cb(&c2c.hists.hists, resort_cl_cb,
	                        perf_session__env(session));
	if (err)
		goto out_mem2node;

Should these error paths also be updated to jump to out_str instead of
out_mem2node to avoid leaking output_str?

>  out_mem2node:
>  	mem2node__exit(&c2c.mem2node);
>  out_session:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831204613.2793526-1-thomas.falcon@intel.com?part=4