All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: "Thomas Weißschuh" <linux@weissschuh.net>
Cc: Willy Tarreau <w@1wt.eu>,
	linux-kernel@vger.kernel.org, Cheng Li <lechain@gmail.com>
Subject: Re: [PATCH v3 next 04/17] selftests/nolibc: Improve reporting of vfprintf() errors
Date: Thu, 26 Feb 2026 10:12:46 +0000	[thread overview]
Message-ID: <20260226101246.07bab7af@pumpkin> (raw)
In-Reply-To: <f705e424-7f0d-45cc-b5e0-6d324ecc9b10@t-8ch.de>

On Wed, 25 Feb 2026 22:56:03 +0100
Thomas Weißschuh <linux@weissschuh.net> 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.

> 
> > 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.
But I can disable it.
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.

> 
> > 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@gmail.com>
> > ---
> > 
> > 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.

> 
> > +
> > +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.

I might just re-post patches to this file.

The later patches will depend on it, but that won't matter if it
get applied.

Where do these patches get applied to?
So I can base new versions on the changes.
I guess they'll get picked up into 'next' fairly quickly.

	David

  reply	other threads:[~2026-02-26 10:12 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-23 10:17 [PATCH v3 next 00/17] Enhance printf() david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 01/17] tools/nolibc: Add _NOLIBC_OPTIMIZER_HIDE_VAR() to compiler.h david.laight.linux
2026-02-25 21:25   ` Thomas Weißschuh
2026-02-25 22:17     ` David Laight
2026-02-25 22:24       ` Thomas Weißschuh
2026-02-23 10:17 ` [PATCH v3 next 02/17] tools/nolibc: Optimise and common up the number to ascii functions david.laight.linux
2026-02-25 21:40   ` Thomas Weißschuh
2026-02-25 22:09     ` David Laight
2026-02-23 10:17 ` [PATCH v3 next 03/17] selftests/nolibc: Fix build with host headers and libc david.laight.linux
2026-02-25 21:24   ` Thomas Weißschuh
2026-02-23 10:17 ` [PATCH v3 next 04/17] selftests/nolibc: Improve reporting of vfprintf() errors david.laight.linux
2026-02-25 21:56   ` Thomas Weißschuh
2026-02-26 10:12     ` David Laight [this message]
2026-02-26 21:39       ` Thomas Weißschuh
2026-02-23 10:17 ` [PATCH v3 next 05/17] tools/nolibc: Implement strerror() in terms of strerror_r() david.laight.linux
2026-02-25 22:09   ` Thomas Weißschuh
2026-02-25 22:58     ` David Laight
2026-02-23 10:17 ` [PATCH v3 next 06/17] tools/nolibc/printf: Change variables 'c' to 'ch' and 'tmpbuf[]' to 'outbuf[]' david.laight.linux
2026-02-25 22:23   ` Thomas Weißschuh
2026-02-23 10:17 ` [PATCH v3 next 07/17] tools/nolibc/printf: Move snprintf length check to callback david.laight.linux
2026-02-25 22:37   ` Thomas Weißschuh
2026-02-25 23:12     ` David Laight
2026-02-26 21:29       ` Thomas Weißschuh
2026-02-26 22:11         ` David Laight
2026-02-23 10:17 ` [PATCH v3 next 08/17] tools/nolibc/printf: Output pad characters in 16 byte chunks david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 09/17] tools/nolibc/printf: Simplify __nolibc_printf() david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 10/17] tools/nolibc/printf: Use goto and reduce indentation david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 11/17] tools/nolibc/printf: Use bit-masks to hold requested flag, length and conversion chars david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 12/17] tools/nolibc/printf: Handle "%s" with the numeric formats david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 13/17] tools/nolibc/printf: Add support for conversion flags david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 14/17] tools/nolibc/printf: Add support for left aligning fields david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 15/17] tools/nolibc/printf: Add support for zero padding and field precision david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 16/17] tools/nolibc/printf: Add support for octal output david.laight.linux
2026-02-23 10:17 ` [PATCH v3 next 17/17] selftests/nolibc: Use printf variable field widths and precisions david.laight.linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260226101246.07bab7af@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=lechain@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.