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

[Thomas Weißschuh]

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

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

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

(...)

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


Thomas