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

[Thomas Weißschuh]

On 2026-02-17 10:48:07+0000, David Laight wrote:
> On Mon, 16 Feb 2026 21:05:40 +0100
> Thomas Weißschuh <linux@xxxxxxxxxxxxxx> wrote:
> 
> > 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.
> 
> I wanted to use different values just in case the test wrote the
> value I'd picked to two adjacent locations.
> I could use a (bad) random value (eg the time(NULL) & 0xff) so that
> an overwrite of the chosen value would be picked up by different run.

What about 'i & 0x0F'?

> > >  	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.
> 
> I definitely wanted the invalid string output on failure.
> Not being able to see why a test failed is a PITA.
> Reporting the length error only makes sense when the output is truncated,
> and the 'truncation length' is a property of expect_vfprintf() not
> the test itself (and I had to increase it for the octal tests).

Printing the full invalid string on failure is great.

My objections was only about the 'if (!c) c = strlen();' shorthand.

> One option is to remove the 'c' parameter completely and require the
> test include the un-truncated output for the truncation tests.
> There is no reason for any of them to be horribly long.

That would work, too. I prefer the return value slightly, as it can be
understood easily to correspond to the function return value.

> Then add a parameter for 'run this test' (like all the other tests)
> so that some tests can be excluded/changed when is_nolibc isn't set.

I left it out intentionally to reduce the amount of parameter soup.
To you expect more conditional testcases to be necessary?
If not a TEST_VFPRINTF_COND() macro used only here looks better to me.


Thomas