Re: [PATCH v2 next 09/11] selftests/nolibc: Improve reporting of vfprintf() errors

[Thomas Weißschuh]

On 2026-02-06 19:11:19+0000, david.laight.linux@xxxxxxxxx wrote:
> From: David Laight <david.laight.linux@xxxxxxxxx>
> 
> Check the string matches before checking the returned length.
> Only print the string once when it matches.
> Normally the length is that of the expected string, make it easier
> to write tests by treating a length of zero as being that of the
> expected output.
> Additionally check that nothing beyond the end is written.
> 
> This also correctly returns 1 (the number of errors) when strcmp()
> fails rather than the return value from strncmp() which is the
> signed difference between the mismatching characters.
> 
> Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> ---
> 
> Changes for v2:
> - Formally patch 5, otherwise unchanged.
> 
>  tools/testing/selftests/nolibc/nolibc-test.c | 27 ++++++++++++++++----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 3c5a226dad3a..9378a1f26c34 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -1567,28 +1567,45 @@ int run_stdlib(int min, int max)
>  
>  static int expect_vfprintf(int llen, int c, const char *expected, const char *fmt, ...)
>  {
> +	unsigned int i;
>  	char buf[100];
>  	va_list args;
>  	ssize_t w;
>  	int ret;
>  
> +	for (i = 0; i < sizeof(buf); i++)
> +		buf[i] = i;

Some of these values are valid ascii characters.
An out-of-bounds write may be missed if the test happens to print the
correct character. Another poison value would avoid this issue.

>  
>  	va_start(args, fmt);
> -	/* Only allow writing 21 bytes, to test truncation */
> +	/* Only allow writing 20 bytes, to test truncation */
>  	w = vsnprintf(buf, 21, fmt, args);
>  	va_end(args);
>  
> +	llen += printf(" \"%s\"", buf);
> +	ret = strcmp(expected, buf);
> +	if (ret) {
> +		llen += printf(" should be \"%s\"", expected);
> +		result(llen, FAIL);
> +		return 1;
> +	}
> +	if (!c)
> +		c = strlen(expected);

This patch does multiple different things again. Please split them up.
Specifically I am not a fan of allowing to pass '0' here.
While it is slightly more convenient when writing the tests,
it increases the opportunity for errors and makes the tests less clear
when reading them.

>  	if (w != c) {
>  		llen += printf(" written(%d) != %d", (int)w, c);
>  		result(llen, FAIL);
>  		return 1;
>  	}
>  
> -	llen += printf(" \"%s\" = \"%s\"", expected, buf);
> -	ret = strncmp(expected, buf, c);
> +	for (i = c + 1; i < sizeof(buf); i++) {
> +		if (buf[i] - i) {

With a fixed poison value the check above wouldn't look this weird.

> +			llen += printf(" overwrote buf[%d] with 0x%x", i, buf[i]);
> +			result(llen, FAIL);
> +			return 1;
> +		}
> +	}
>  
> -	result(llen, ret ? FAIL : OK);
> -	return ret;
> +	result(llen, OK);
> +	return 0;
>  }
>  
>  static int test_scanf(void)
> -- 
> 2.39.5
>