Re: [PATCH v1 02/12] perf test: Truncate test description to fit terminal width

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Subtest descriptions are unnecessarily truncated because the alignment padding width (`subw`) is incorrectly used as the maximum string precision.
- [Medium] Test descriptions are unconditionally truncated to 45 characters when the output is redirected to a file or pipe, degrading standard log outputs.
--

commit 4e5040303fa86a90233f1836bc00692228c7e994
Author: Ian Rogers <irogers@google.com>

perf test: Truncate test description to fit terminal width

This commit dynamically queries the terminal width to truncate printed test
descriptions. This prevents long names from wrapping around the terminal
and corrupting the parallel test harness's progress lines.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index afc06cec49546..51484e84d7c15 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
[ ... ]
> @@ -413,19 +417,52 @@ static char *xml_escape(const char *str)
>  	return res ? res : strdup("");
>  }
>  
> +static int get_max_desc_width(int width)
> +{
> +	struct winsize ws;
> +	int cols = 80;
> +	int max_desc_width;
> +
> +	get_term_dimensions(&ws);
> +	if (ws.ws_col > 0)
> +		cols = ws.ws_col;
> +
> +	/*
> +	 * Limit description width to fit on a single line. We subtract 35
> +	 * columns of headroom to allocate space for:
> +	 * - The suite index prefix: e.g. " 10.100:" (9 characters).
> +	 * - The colon separator and spaces: " : " (3 characters).
> +	 * - The longest status results: e.g. "Skip (some metrics failed)" (26 characters)
> +	 *   or "Running (XX active)" (20 characters).
> +	 *
> +	 * A minimum description width of 10 is enforced to ensure names are
> +	 * legible even on very narrow consoles.
> +	 */
> +	max_desc_width = cols - 35;
> +	if (max_desc_width < 10)
> +		max_desc_width = 10;
> +
> +	return width > max_desc_width ? max_desc_width : width;
> +}
> +
>  static int print_test_result(struct test_suite *t, int curr_suite, int curr_test_case,
>  			     int result, int width, int running,
>  			     const char *err_output, double elapsed)
>  {
> +	width = get_max_desc_width(width);

[Severity: Medium]
Will this unconditionally truncate test descriptions to 45 characters when
output is redirected to a file or pipe?

When a TTY isn't present, get_term_dimensions() fails and cols defaults to
80, making max_desc_width evaluate to 45. Is it intended to enforce this
truncation on non-interactive text logs where line wrapping isn't an issue?

> +
>  	if (test_suite__num_test_cases(t) > 1) {
>  		char prefix[32];
>  		int len = snprintf(prefix, sizeof(prefix), "%3d.%1d:",
>  				   curr_suite + 1, curr_test_case + 1);
>  		int subw = len >= 4 ? width + 4 - len : width;
>  
> -		pr_info("%s %-*s:", prefix, subw, test_description(t, curr_test_case));
> -	} else
> -		pr_info("%3d: %-*s:", curr_suite + 1, width, test_description(t, curr_test_case));
> +		pr_info("%s %-*.*s:", prefix, subw, subw,
> +			test_description(t, curr_test_case));

[Severity: Medium]
Does using subw as the string precision here cause subtest descriptions to
be unnecessarily truncated?

subw is calculated as width + 4 - len, which makes it slightly smaller than
the global width to accommodate the longer subtest prefix. If a subtest
description length is close to the maximum test description length, it
appears this format will forcefully chop off the end of the description,
even on wide terminals where plenty of space exists.

> +	} else {
> +		pr_info("%3d: %-*.*s:", curr_suite + 1, width, width,
> +			test_description(t, curr_test_case));
> +	}

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