Re: [PATCH v2 next 04/11] tools/nolibc/printf: Output pad characters in 16 byte chunks

[David Laight]

On Mon, 16 Feb 2026 20:30:27 +0100
Thomas Weißschuh <linux@xxxxxxxxxxxxxx> wrote:

> On 2026-02-06 19:11:14+0000, david.laight.linux@xxxxxxxxx wrote:
> > From: David Laight <david.laight.linux@xxxxxxxxx>
> > 
> > Simple to do and saves calls to the callback function.
> > 
> > Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> > ---
> > 
> > Changes for v2:
> > Formally patch 3, unchanged.
> > 
> >  tools/include/nolibc/stdio.h | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > index 552f09d51d82..c044a6b3babe 100644
> > --- a/tools/include/nolibc/stdio.h
> > +++ b/tools/include/nolibc/stdio.h
> > @@ -355,10 +355,12 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  			outstr = fmt;
> >  			len = ofs - 1;
> >  		flush_str:
> > -			while (width-- > len) {
> > -				if (cb(state, " ", 1) != 0)
> > +			while (width > len) {
> > +				unsigned int pad_len = ((width - len - 1) & 15) + 1;  
> 
> So this is a modulo, which does not return 0.
> While I am sure you and Willy recognize this instantly, I had to squint a bit.
> While I understand that a regular modulo operator might be problematic
> on some architectures, does it also apply to constant module values?

The modulo would return 0..15, I wanted 1..16 hence the -1 and +1.
Perhaps another comment.

> The compiler could do this on its own.  If not, we should have a macro.
> 
> I am wondering why it is a modulo and not some sort of max()...

It doesn't require a conditional of any sort.
Basically it does the short transfer first followed by the full length ones.
Whereas a min/max would do the large ones first and the remainder at the end.
The effect is basically the same.

> 
> Could you move the tests for the new functionality into the patches that
> introduce that functionality?

Can do, I was thinking you'd prefer separate patches for the code changes
and test changes and didn't want to generate a stupid number of patches.
It is bad enough already :-)

> That would make it easier to review and
> play with the code. Also it would allow to selectively apply the patches.
> For example I get compiler warnings/errors in the later patches, so I
> the full array of new tests will fail if I skip these later patches.

I've been applying the patch so the tests and then doing a 'git reset' on
it and not worrying about the tests that fail.

	David

> 
> > +				width -= pad_len;
> > +				written += pad_len;
> > +				if (cb(state, "                ", pad_len) != 0)
> >  					return -1;
> > -				written += 1;
> >  			}
> >  			if (cb(state, outstr, len) != 0)
> >  				return -1;
> > -- 
> > 2.39.5
> >