Re: [PATCH v4 8/9] perf ui hists: Remove duplicated thread in popup_action

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Low] Missing `<limits.h>` inclusion causes a musl libc build failure due to `PATH_MAX` usage.
--

--- Patch [8]: [PATCH v4 8/9] perf ui hists: Remove duplicated thread in popup_action ---
commit cf81872962e43b7c33b75762f7881111734c277c
Author: Ian Rogers <irogers@google.com>

perf ui hists: Remove duplicated thread in popup_action

struct popup_action has a thread but this is duplicated in the
map_symbol. Remove the non-map_symbol version so that there's only ever
1 thread with a popup_action.

> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index d61ebe4f6a6a4..0713e3ddcf1f0 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2455,7 +2455,6 @@ static int switch_data_file(void)

[Severity: Low]
This isn't a bug introduced by this patch, but should limits.h be explicitly
included at the top of tools/perf/ui/browsers/hists.c?

Looking at switch_data_file(), it uses the POSIX constant PATH_MAX:

static int switch_data_file(void)
{
        char path[PATH_MAX];
        ...

Does omitting the explicit inclusion of limits.h cause build failures on musl
libc systems, which do not implicitly include headers in the same way glibc
does?

>  
>  struct popup_action {
>  	unsigned long		time;
> -	struct thread 		*thread;
>  	int (*fn)(struct hist_browser *browser, struct popup_action *act);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710024952.1668911-1-irogers@google.com?part=8