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

[Thomas Weißschuh]

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

Could you move the tests for the new functionality into the patches that
introduce that functionality? 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.

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