Re: [PATCH v1 6/7] perf ui hist: Format group headers iteratively based on proportional visual allocations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Truncation of the `buf` array causes severe visual misalignment of columns when printing group event headers.
--

--- Patch [6]: [PATCH v1 6/7] perf ui hist: Format group headers iteratively based on proportional visual allocations ---
commit fb91b42310113a183352643a9b299c122e218ea8
Author: Ian Rogers <irogers@google.com>

perf ui hist: Format group headers iteratively based on proportional visual allocations

Forcefully calculate proportional visual allocations only natively for
the 'Total' merge target header instead of uniformly ballooning array
spacings. Unpack strings directly into the primary column width fn.

> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index 09f0baa808e0a..02c188100b098 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c

[ ... ]

> @@ -421,8 +458,25 @@ static int hpp__header_fn(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
>  	int len = hpp__width_fn(fmt, hpp, hists);
>  	const char *hdr = "";
>  
> -	if (line == hists->hpp_list->nr_header_lines - 1)
> +	if (line == hists->hpp_list->nr_header_lines - 1) {
>  		hdr = fmt->name;
> +		if (hists->merge_entries && &fmt->list == hists->hpp_list->fields.next) {
> +			char buf[1024];
> +			int w = 0;
> +			int f_len = fmt->user_len ?: fmt->len;
> +			struct evsel *pos, *evsel = hists_to_evsel(hists);
> +
> +			w += scnprintf(buf + w, sizeof(buf) - w, "%*.*s", f_len, f_len, fmt->name);
> +			for_each_group_evsel(pos, evsel) {
> +				if (symbol_conf.skip_empty &&
> +			    evsel__hists(pos)->stats.nr_samples == 0)
> +					continue;
> +				w += scnprintf(buf + w, sizeof(buf) - w, " %*.*s",
> +					       f_len - 1, f_len - 1, evsel__name(pos));
> +			}
> +			return scnprintf(hpp->buf, hpp->size, "%*s", len, buf);

[Severity: Medium]
If the event group has many members (e.g., from a hybrid merge), can the
concatenated header string exceed the 1024 byte limit of buf array?

If the array truncates the string, but len is calculated from the total width
of all members, won't this cause the %*s specifier to right-justify the
truncated string within the larger len field?

Could this result in padding the string with leading spaces and cause severe
visual misalignment of columns in the UI output?

> +		}
> +	}
>  
>  	return scnprintf(hpp->buf, hpp->size, "%*s", len, hdr);
>  }

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