Re: [PATCH 17/23] tools/nolibc/printf: Prepend sign to converted number

[Willy Tarreau]

On Mon, Mar 02, 2026 at 10:18:09AM +0000, david.laight.linux@xxxxxxxxx wrote:
> From: David Laight <david.laight.linux@xxxxxxxxx>
> 
> Instead of appending the converted number to the sign, convert first
> and then prepend the sign (or "0x").
> Use the length returned by u64toh_r() instead of calling strlen().
> 
> Needed so that zero padding can be inserted between the sign and digits
> in an upcoming patch.
> 
> Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> ---
> 
> Changes for v4:
> - Split from the patch that supported modifiers " +#".
> 
>  tools/include/nolibc/stdio.h | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index 484432ca87d5..fb310b7d023f 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
> @@ -345,9 +345,10 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  	long long signed_v;
>  	int written, width, len;
>  	unsigned int flags, ch_flag;
> -	char outbuf[21];
> +	char outbuf[2 + 22 + 1];
>  	char *out;
>  	const char *outstr;
> +	unsigned int sign_prefix;
>  
>  	written = 0;
>  	while (1) {
> @@ -446,32 +447,47 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  				goto do_strlen_output;
>  			}
>  
> -			out = outbuf;
> +			/* The 'sign_prefix' can be zero, one or two ("0x") characters. */
> +			sign_prefix = 0;

It would help to mention that last chars are on the lowest bits, because
it's not totally intuitive that these will be consumed in reverse order
at the end. Also even if obvious once you see it, mentioning that we stop
on the first zero byte would help reviewers ;-)

>  			if (_NOLIBC_PF_FLAGS_CONTAIN(ch_flag, 'd', 'i')) {
>  				/* "%d" and "%i" - signed decimal numbers. */
>  				if (signed_v < 0) {
> -					*out++ = '-';
> +					sign_prefix = '-';
>  					v = -(signed_v + 1);
>  					v++;
>  				}
>  			}
>  
> +			/* The value is converted offset into the buffer so that
> +			 * the sign/prefix can be added in front.
> +			 * The longest digit string is 22 + 1 for octal conversions, the
> +			 * space is reserved even though octal isn't currently supported.
> +			 */
> +			out = outbuf + 2;
> +
>  			/* Convert the number to ascii in the required base. */
>  			if (_NOLIBC_PF_FLAGS_CONTAIN(ch_flag, 'd', 'i', 'u')) {
>  				/* Base 10 */
> -				u64toa_r(v, out);
> +				len = u64toa_r(v, out);
>  			} else {
>  				/* Base 16 */
>  				if (_NOLIBC_PF_FLAGS_CONTAIN(ch_flag, 'p')) {
> -					*(out++) = '0';
> -					*(out++) = 'x';
> +					/* "%p" needs "0x" prepending. */
> +					sign_prefix = 'x' | '0' << 8;

This one could have been more readable in the other order:

					sign_prefix = '0' << 8 | 'x';

>  				}
> -				u64toh_r(v, out);
> +				len = u64toh_r(v, out);
>  			}
>  
> -			outstr = outbuf;
> -			goto do_strlen_output;
> +			/* Add the 0, 1 or 2 ("0x") sign/prefix characters at the front. */
> +			for (; sign_prefix; sign_prefix >>= 8) {
> +				/* Force gcc to increment len inside the loop. */
> +				_NOLIBC_OPTIMIZER_HIDE_VAR(len);
> +				len++;
> +				*--out = sign_prefix;
> +			}
> +			outstr = out;
> +			goto do_output;
>  		}
>  
>  		if (ch == 'm') {

Overall I'm fine with this and my comments only apply to any potential
respin.

Acked-by: Willy Tarreau <w@xxxxxx>

Willy