Re: [PATCH v1 4/8] perf annotate-data: Show the sample count in the data-type browser

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but child nodes and their 'hists' arrays are leaked during browser entries deletion.
- [Medium] This problem wasn't introduced by this patch, but there are missing POSIX libc header inclusions for `snprintf`, `calloc`, and `free`.
--

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

perf annotate-data: Show the sample count in the data-type browser

The data-type browser has a samples view, selected with -n (or with
annotate.show_nr_samples), in which browser__write_overhead() prints a
local nr_samples variable that is initialized to zero and never
updated, so every member is listed as having no samples while the
period and percent columns for the same entry are filled in.

Print the histogram entry's own count instead.

This predates the load/store counter split, so fix it ahead of that
patch: the split then only has to adapt a line that is already correct,
and this fix can be picked on its own.

Fixes: d001c7a7f4736743 ("perf annotate-data: Add hist_entry__annotate_data_tui()")
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

> diff --git a/tools/perf/ui/browsers/annotate-data.c b/tools/perf/ui/browsers/annotate-data.c
> index aa8c89fe2e82c..1080ed1a40d26 100644
> --- a/tools/perf/ui/browsers/annotate-data.c
> +++ b/tools/perf/ui/browsers/annotate-data.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but child nodes and their hists
arrays are leaked when browser entries are deleted:

tools/perf/ui/browsers/annotate-data.c:annotated_data_browser__delete_entries() {
	list_for_each_entry_safe(pos, tmp, &browser->entries, node) {
		list_del_init(&pos->node);
		zfree(&pos->hists);
		free(pos);
	}
}

The loop exclusively iterates over the top-level nodes in the browser->entries
list. Does this leak the memory for all nested child nodes in pos->children and
their associated hists arrays?

If a user repeatedly opens and closes the data type annotation TUI, could this
unbounded leak eventually exhaust system memory and crash the tool during a
long-lived interactive session?


[Severity: Medium]
This is a pre-existing issue, but does this file fail to build under musl libc?

tools/perf/ui/browsers/annotate-data.c
	#include <inttypes.h>
	#include <string.h>
	#include <linux/zalloc.h>
	#include <sys/ttydefaults.h>

The code uses functions from stdio.h (snprintf) and stdlib.h (calloc, free)
without explicitly including these headers. Could we add these missing POSIX
libc header inclusions to ensure musl compilation compatibility?

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