Re: [PATCH v2 6/6] perf c2c: print memory region data with stdio output

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The perf c2c tool structure omits the `.feature` callback, breaking `perf_env` initialization in pipe mode.
- [Low] Implicit reliance on POSIX libc headers violates musl build compatibility rules (missing `<stdio.h>`).
--

--- Patch [6]: [PATCH v2 6/6] perf c2c: print memory region data with stdio output ---
commit 210b29f2c6ae9c7cc910fd9994d1e0b92078eec2
Author: Thomas Falcon <thomas.falcon@intel.com>

    perf c2c: print memory region data with stdio output
    
    Print memory range data in perf-c2c subcommand. Only available in stdio
    mode currently. If memory ranges are not supported or present, print
    nothing.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index cba2e9f21ae7c..68348a93bd331 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -2634,6 +2634,34 @@ static void print_c2c_info(FILE *out, struct perf_session *session)
>  	fprintf(out, "  Cacheline data grouping           : %s\n", c2c.cl_sort);
>  }
>  
> +
> +static void print_memory_ranges_info(FILE *out, struct perf_session *session)
> +{

[Severity: Low]
Does this code rely on implicit header inclusion?

The new print_memory_ranges_info() function uses FILE and fprintf, but the
patch doesn't explicitly include <stdio.h>. Can this cause the code to fail to
compile under musl libc due to missing declarations?

> +	struct perf_env *env = perf_session__env(session);
> +	int nr_ranges = 0;
> +
> +	if (!perf_header__has_feat(&session->header, HEADER_MEMORY_RANGES))
> +		return;
> +	nr_ranges = env->nr_memory_ranges;
> +	if (nr_ranges == 0) {
> +		pr_debug("No memory ranges found, skipping\n");
> +		return;
> +	}
> +
> +	fprintf(out, "\n");
> +	fprintf(out, "=================================================\n");
> +	fprintf(out, "                  Memory Ranges                  \n");
> +	fprintf(out, "=================================================\n");
> +
> +	for (int i = 0; i < nr_ranges; i++) {
> +		struct memory_range *r = &env->memory_ranges[i];
> +
> +		fprintf(out, "Range %d: [0x%016" PRIx64 "-0x%016" PRIx64 "] Node %d, local region id %u, remote region id %u\n",
> +			i, r->base, r->base + r->length - 1, r->node,
> +			r->local_region_id, r->remote_region_id);
> +	}
> +}

[Severity: Medium]
Will this silently fail to display memory ranges when perf is run in pipe
mode?

The tool initializes its callbacks in perf_c2c__report() like this:

	perf_tool__init(&c2c.tool, /*ordered_events=*/true);
	c2c.tool.sample		= process_sample_event;
	c2c.tool.mmap		= perf_event__process_mmap;

Because the .feature callback is missing here, feature events are dropped in
pipe mode, so perf_env is not populated with memory ranges. Although the
callback omission was pre-existing, doesn't it actively break the
functionality introduced in this patch?

[ ... ]

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