public inbox for linux-kernel@vger.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 v2 next 09/11] selftests/nolibc: Improve reporting of vfprintf() errors
Date: Tue, 17 Feb 2026 10:48:07 +0000	[thread overview]
Message-ID: <20260217104807.7eb7752a@pumpkin> (raw)
In-Reply-To: <af000aa2-df95-4205-a1c9-d17e03d22aee@t-8ch.de>

On Mon, 16 Feb 2026 21:05:40 +0100
Thomas Weißschuh <linux@weissschuh.net> wrote:

> On 2026-02-06 19:11:19+0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > 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@gmail.com>
> > ---
> > 
> > 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.

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

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.

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.

	David

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


  reply	other threads:[~2026-02-17 10:48 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-06 19:11 [PATCH v2 next 00/11] tools/nolibc: Enhance printf() david.laight.linux
2026-02-06 19:11 ` [PATCH v2 next 01/11] tools/nolibc/printf: Change variable used for format chars from 'c' to 'ch' david.laight.linux
2026-02-07 18:51   ` Willy Tarreau
2026-02-16 18:52   ` Thomas Weißschuh
2026-02-06 19:11 ` [PATCH v2 next 02/11] tools/nolibc/printf: Move snprintf length check to callback david.laight.linux
2026-02-07 19:12   ` Willy Tarreau
2026-02-07 23:28     ` David Laight
2026-02-08 15:12       ` Willy Tarreau
2026-02-08 22:49         ` David Laight
2026-02-06 19:11 ` [PATCH v2 next 03/11] tools/nolibc/printf: Add buffering to vfprintf() callback david.laight.linux
2026-02-07 19:29   ` Willy Tarreau
2026-02-07 23:36     ` David Laight
2026-02-16 19:07       ` Thomas Weißschuh
2026-02-17 11:51         ` David Laight
2026-02-18 17:52           ` Thomas Weißschuh
2026-02-06 19:11 ` [PATCH v2 next 04/11] tools/nolibc/printf: Output pad characters in 16 byte chunks david.laight.linux
2026-02-07 19:38   ` Willy Tarreau
2026-02-07 23:43     ` David Laight
2026-02-08 15:14       ` Willy Tarreau
2026-02-16 19:30   ` Thomas Weißschuh
2026-02-16 22:29     ` David Laight
2026-02-18 17:30       ` Thomas Weißschuh
2026-02-06 19:11 ` [PATCH v2 next 05/11] tools/nolibc/printf: Simplify __nolibc_printf() david.laight.linux
2026-02-07 20:05   ` Willy Tarreau
2026-02-07 23:50     ` David Laight
2026-02-08 12:20       ` David Laight
2026-02-08 14:44         ` Willy Tarreau
2026-02-08 16:54           ` David Laight
2026-02-08 17:06             ` Willy Tarreau
2026-02-06 19:11 ` [PATCH v2 next 06/11] tools/nolibc/printf: Use bit-masks to hold requested flag, length and conversion chars david.laight.linux
2026-02-08 15:22   ` Willy Tarreau
2026-02-16 19:52   ` Thomas Weißschuh
2026-02-16 22:47     ` David Laight
2026-02-18 17:36       ` Thomas Weißschuh
2026-02-18 22:57         ` David Laight
2026-02-06 19:11 ` [PATCH v2 next 07/11] tools/nolibc/printf: Add support for conversion flags "#- +" and format "%X" david.laight.linux
2026-02-08 15:47   ` Willy Tarreau
2026-02-08 17:14     ` David Laight
2026-02-08 16:06   ` Willy Tarreau
2026-02-16 19:57   ` Thomas Weißschuh
2026-02-16 22:50     ` David Laight
2026-02-18 17:39       ` Thomas Weißschuh
2026-02-16 20:11   ` Thomas Weißschuh
2026-02-16 22:52     ` David Laight
2026-02-06 19:11 ` [PATCH v2 next 08/11] tools/nolibc/printf: Add support for zero padding and field precision david.laight.linux
2026-02-08 16:16   ` Willy Tarreau
2026-02-08 17:31     ` David Laight
2026-02-06 19:11 ` [PATCH v2 next 09/11] selftests/nolibc: Improve reporting of vfprintf() errors david.laight.linux
2026-02-16 20:05   ` Thomas Weißschuh
2026-02-17 10:48     ` David Laight [this message]
2026-02-18 17:48       ` Thomas Weißschuh
2026-02-06 19:11 ` [PATCH v2 next 10/11] selftests/nolibc: Increase coverage of printf format tests david.laight.linux
2026-02-16 20:14   ` Thomas Weißschuh
2026-02-16 20:23   ` Thomas Weißschuh
2026-02-16 22:54     ` David Laight
2026-02-18 17:41       ` Thomas Weißschuh
2026-02-06 19:11 ` [PATCH v2 next 11/11] selftests/nolibc: Use printf("%.*s", n, "") to align output david.laight.linux
2026-02-08 16:20   ` Willy Tarreau
2026-02-16 20:22   ` Thomas Weißschuh
2026-02-06 21:36 ` [PATCH v2 next 00/11] tools/nolibc: Enhance printf() David Laight

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=20260217104807.7eb7752a@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox