Re: [PATCH 1/1] tools/nolibc/printf: Support negative variable width and precision

[David Laight]

On Wed, 25 Mar 2026 21:37:11 +0100
Thomas Weißschuh <linux@xxxxxxxxxxxxxx> wrote:

> Hi David,
> 
> thanks again for your patch!
> 
> On 2026-03-23 11:22:47+0000, david.laight.linux@xxxxxxxxx wrote:
> > From: David Laight <david.laight.linux@xxxxxxxxx>
> > 
> > For (eg) "%*.*s" treat a negative field width as a request to left align
> > the output (the same as the '-' flag), and a negative precision to
> > request the default precision.  
> 
> This makes sense, so far so good.
> 
> > Set the default precision to -1 (not INT_MAX) and add explicit checks
> > to the string handling for negative values (makes the tet unsigned).  
> 
> 'tet'?

test - I wanted to say that the test:
	len = precision < 0 || precision >= 6 ? 6 : 0;
isn't actually a double comparison.
Perhaps it isn't needed.

> 
> > For numeric output check for 'precision >= 0' instead of testing
> > _NOLIBC_PF_FLAGS_CONTAIN(flags, '.').
> > This needs an inverted test, some extra goto and removes an indentation.
> > The changed conditionals fix printf("%0-#o", 0) - but '0' and '-' shouldn't
> > both be specified.  
> 
> Is this also related to the negative field width as described above?
> If yes, could you explain how? If not, please split it out.
> In general, the smaller the patches, the easier the review.

Everything except the 'if (width < 0)' test is one change.
It also changes v5 15/17, I did it as a delta (rather than a replacement
patch) because of the later changes.

The fix for printf("%0-#o", 0) (without these changes it generates "00")
happens because of the way the conditionals and gotos change.
Patch v5 15/17 that added zero padding and field precision had two
'goto prepend_sign', patch 16 (add octal) needs them to go opposite
sides of the 'if (sign_prefix == *out)' test.
With the changed conditionals you need both labels or ("%#o", 0)
always goes wrong.

> 
> > Best viewed with 'git diff -b' after being commited.  
> 
> This should go after '---'. No reason on its own to resend, though.
> 
> > Additional test cases added.  
> 
> No need to mention that, it is obvious from the diff :-)
> 
> > Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> > ---
> > 
> > I missed this bit in the earlier patches.
> > Size wise it is pretty neutral. It really seems to depend on how many registers
> > get saved across the call to _nolibc_u64toa_base() - gcc doesn't seem to use
> > the correct registers to avoid spills.
> > 
> > I did look at whether making 'width' negative at the top was better than
> > keeping a '-' flag - but it bloats things because you need the absolute value
> > at the bottom.
> > 
> >  tools/include/nolibc/stdio.h                 | 68 +++++++++++---------
> >  tools/testing/selftests/nolibc/nolibc-test.c |  5 +-
> >  2 files changed, 41 insertions(+), 32 deletions(-)
> > 
> > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > index 8f7e1948a651..b6d14a58cfe7 100644
> > --- a/tools/include/nolibc/stdio.h
> > +++ b/tools/include/nolibc/stdio.h
> > @@ -347,6 +347,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  	char *out;
> >  	const char *outstr;
> >  	unsigned int sign_prefix;
> > +	int got_width;  
> 
> bool?

There weren't any others in this file and I'm 'old school'...

	David

> 
> (...)
> 
> Otherwise looks good from what I understand.
> But smaller patches would really give me more confidence.
> 
> 
> Thomas