Re: [PATCH v3 next 04/17] selftests/nolibc: Improve reporting of vfprintf() errors

[Thomas Weißschuh]

On 2026-02-26 10:12:46+0000, David Laight wrote:
> On Wed, 25 Feb 2026 22:56:03 +0100
> Thomas Weißschuh <linux@xxxxxxxxxxxxxx> wrote:
> 
> ... 
> > > Increase the size limit from 20 to 25 characters, changing the tests to
> > > match. This is needed to test octal conversions later on.  
> > 
> > Then please do this right before the addition of the octal conversion.
> 
> I also kept hitting the limit trying to write other tests.
> It is very easy to hit 20 chars when testing precision (three %6.2d is
> too long)
> Although I think the tests were written with that change done later.
> I put it early so that I wouldn't have to change new tests that
> tested truncation.
>
> Perhaps I should justify the change because it lets tests check
> longer data, not just octal?
> I will then use longer test output in some of the other patches.

That sounds good. But please in a dedicated patch.

> > > Append a '+' to the printed output (after the final ") when the output
> > > is truncated.
> > > 
> > > Additionally check that nothing beyond the end is written.
> > > The "width_trunc" test (#14) now fails because e90ce42e81381
> > > ("tools/nolibc: implement width padding in printf()") doesn't
> > > correctly update the space in the buffer when adding pad characters.
> > > This will be addressed in a later patch.  
> > 
> > The build bots will yell at us for this.
> > Instead mark the test as skipped until it is fixed.
> 
> The build bots won't bleat - it is a run-time error.

LKP also does some selftests. It will break sometime.
Also I run the tests for each commit and don't want broken
intermediate states.

> But I can disable it.

Can we not just move the fix to printf before this commit?
Then the problem just goes away.

> I'm not sure there is a #define for 'broken' and there isn't
> room for comments on the test cases.
> So it might just be a literal 0.

If it needs to be disabled use "0 /* FIXME */" and mention it in the
commit message. But reordering the commits would be better.

> > > Also correctly return 1 (the number of errors) when strcmp()
> > > fails rather than the return value from strncmp() which is the
> > > signed difference between the mismatching characters.  
> > 
> > Please split these different steps into dedicated commits.
> > Yes, the testcases are going to be rewritten a bunch of times,
> > but that does not matter.
> > 
> > > Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> > > ---
> > > 
> > > For v3:
> > > - Patch 9 in v2, patch 5 in v1..
> > > - Increase the size limit to 25 in preparation for testing octal.
> > > - Change the way truncated fprintf() are handled.
> > > - Allow for tests being skipped.
> > > - Use a fixed value (0xa5) for the canary when detecting overwrites.
> > > 
> > >  tools/testing/selftests/nolibc/nolibc-test.c | 91 ++++++++++++++------
> > >  1 file changed, 64 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > > index 0e8b3b9a86ef..029ed63e1ae4 100644
> > > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > > @@ -1660,33 +1660,70 @@ int run_stdlib(int min, int max)
> > >  	return ret;
> > >  }
> > >  
> > > -#define EXPECT_VFPRINTF(c, expected, fmt, ...)				\
> > > -	ret += expect_vfprintf(llen, c, expected, fmt, ##__VA_ARGS__)
> > > +#define EXPECT_VFPRINTF(cond, expected, fmt, ...)				\
> > > +	ret += expect_vfprintf(llen, cond, expected, fmt, ##__VA_ARGS__)
> > >  
> > > -static int expect_vfprintf(int llen, int c, const char *expected, const char *fmt, ...)
> > > +#define VFPRINTF_LEN 25  
> > 
> > This is only used within expect_vfprintf(), so it can be a local variable.
> 
> I used it to size buf[] so it needs to be an 'integer constant expression'.
> The only simple way to do that is with a #define - which isn't scoped.

Yeah, ok.

> > > +
> > > +static int expect_vfprintf(int llen, int cond, const char *expected, const char *fmt, ...)
> > >  {
> > > -	char buf[100];
> > > +	ssize_t written, expected_len;
> > > +	char buf[VFPRINTF_LEN + 80];
> > > +	unsigned int cmp_len;
> > >  	va_list args;
> > > -	ssize_t w;
> > > -	int ret;
> > >  
> > > +	if (!cond) {
> > > +		result(llen, SKIPPED);
> > > +		return 0;
> > > +	}  
> > 
> > The other EXPECT_*() macros evaluate the condition in the macro, not the
> > corresponding function. I'm not entirely sure why that is, but please
> > keep it consistent.
> 
> Most of the other ones call the function in the #define and just report
> the success/fail in the function. The SKIP test has to be before the
> function call - so has to be in the #define.
> For vfprintf (should be snprintf) the test is in the function.
> So really it should be TEST_SNPRINTF().
> The only other similar one is EXPECT_STRTOX().
> 
> But I can change it for consistency.

Please do.

> I might just re-post patches to this file.

Please repost the whole series. Or just a small series with the already
reviewed bits (patches 1-9) which I can apply easily and wait for proper
review of the big new stuff.

> The later patches will depend on it, but that won't matter if it
> get applied.
> 
> Where do these patches get applied to?

https://git.kernel.org/pub/scm/linux/kernel/git/nolibc/linux-nolibc.git/log/?h=for-next

> So I can base new versions on the changes.
> I guess they'll get picked up into 'next' fairly quickly.

They are already in today's -next.


Thomas