Re: [PATCH v4 next 13/23] tools/nolibc/printf: Use goto and reduce indentation

[Willy Tarreau]

On Mon, Mar 02, 2026 at 10:18:05AM +0000, david.laight.linux@xxxxxxxxx wrote:
> From: David Laight <david.laight.linux@xxxxxxxxx>
> 
> Upcoming changes will need to use goto to jump to the code that
> outputs characters.
> Use 'goto do_output' to output a known number of characters.
> Use 'goto do_strlen_output' to output a '\0' terminated string.
> 
> Removes a level of indentation from the format processing code.

Nice! I, too, prefer this more explicit way of transitionning between
states. For others, the patch is best reviewed with "git show -b" or
"git diff -b" after applying it (67 lines instead of 200).

David, when changing code indent like this, I often like to suggest
doing this in the commit messages. I'm saying this in case there's
a further respin.

> Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>

Acked-by: Willy Tarreau <w@xxxxxx>

Willy

> ---
> 
> For v4:
> - Output a single '%' from the format string.
> 
> New patch for v3.
> Makes the final code look better and there is less to change if done early.
> 
>  tools/include/nolibc/stdio.h | 170 +++++++++++++++++++----------------
>  1 file changed, 92 insertions(+), 78 deletions(-)
> 
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index e0b7ff537b14..13fe6c4d7f58 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
> @@ -329,103 +329,117 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  				fmt++;
>  			/* Output characters from the format string. */
>  			len = fmt - outstr;
> -		} else {
> -			/* we're in a format sequence */
> +			goto do_output;
> +		}
>  
> -			ch = *fmt++;
> +		/* we're in a format sequence */
>  
> -			/* width */
> -			while (ch >= '0' && ch <= '9') {
> -				width *= 10;
> -				width += ch - '0';
> +		ch = *fmt++;
>  
> -				ch = *fmt++;
> -			}
> +		/* width */
> +		while (ch >= '0' && ch <= '9') {
> +			width *= 10;
> +			width += ch - '0';
> +
> +			ch = *fmt++;
> +		}
>  
> -			/* Length modifiers */
> +		/* Length modifiers */
> +		if (ch == 'l') {
> +			lpref = 1;
> +			ch = *fmt++;
>  			if (ch == 'l') {
> -				lpref = 1;
> -				ch = *fmt++;
> -				if (ch == 'l') {
> -					lpref = 2;
> -					ch = *fmt++;
> -				}
> -			} else if (ch == 'j') {
> -				/* intmax_t is long long */
>  				lpref = 2;
>  				ch = *fmt++;
> -			} else {
> -				lpref = 0;
>  			}
> +		} else if (ch == 'j') {
> +			/* intmax_t is long long */
> +			lpref = 2;
> +			ch = *fmt++;
> +		} else {
> +			lpref = 0;
> +		}
>  
> -			if (ch == 'c' || ch == 'd' || ch == 'u' || ch == 'x' || ch == 'p') {
> -				char *out = outbuf;
> +		if (ch == 'c' || ch == 'd' || ch == 'u' || ch == 'x' || ch == 'p') {
> +			char *out = outbuf;
>  
> -				if (ch == 'p')
> +			if (ch == 'p')
> +				v = va_arg(args, unsigned long);
> +			else if (lpref) {
> +				if (lpref > 1)
> +					v = va_arg(args, unsigned long long);
> +				else
>  					v = va_arg(args, unsigned long);
> -				else if (lpref) {
> -					if (lpref > 1)
> -						v = va_arg(args, unsigned long long);
> -					else
> -						v = va_arg(args, unsigned long);
> -				} else
> -					v = va_arg(args, unsigned int);
> -
> -				if (ch == 'd') {
> -					/* sign-extend the value */
> -					if (lpref == 0)
> -						v = (long long)(int)v;
> -					else if (lpref == 1)
> -						v = (long long)(long)v;
> -				}
> +			} else
> +				v = va_arg(args, unsigned int);
>  
> -				switch (ch) {
> -				case 'c':
> -					out[0] = v;
> -					out[1] = 0;
> -					break;
> -				case 'd':
> -					i64toa_r(v, out);
> -					break;
> -				case 'u':
> -					u64toa_r(v, out);
> -					break;
> -				case 'p':
> -					*(out++) = '0';
> -					*(out++) = 'x';
> -					__nolibc_fallthrough;
> -				default: /* 'x' and 'p' above */
> -					u64toh_r(v, out);
> -					break;
> -				}
> -				outstr = outbuf;
> +			if (ch == 'd') {
> +				/* sign-extend the value */
> +				if (lpref == 0)
> +					v = (long long)(int)v;
> +				else if (lpref == 1)
> +					v = (long long)(long)v;
>  			}
> -			else if (ch == 's') {
> -				outstr = va_arg(args, char *);
> -				if (!outstr)
> -					outstr="(null)";
> +
> +			switch (ch) {
> +			case 'c':
> +				out[0] = v;
> +				out[1] = 0;
> +				break;
> +			case 'd':
> +				i64toa_r(v, out);
> +				break;
> +			case 'u':
> +				u64toa_r(v, out);
> +				break;
> +			case 'p':
> +				*(out++) = '0';
> +				*(out++) = 'x';
> +				__nolibc_fallthrough;
> +			default: /* 'x' and 'p' above */
> +				u64toh_r(v, out);
> +				break;
>  			}
> -			else if (ch == 'm') {
> +			outstr = outbuf;
> +			goto do_strlen_output;
> +		}
> +
> +		if (ch == 's') {
> +			outstr = va_arg(args, char *);
> +			if (!outstr)
> +				outstr="(null)";
> +			goto do_strlen_output;
> +		}
> +
> +		if (ch == 'm') {
>  #ifdef NOLIBC_IGNORE_ERRNO
> -				outstr = "unknown error";
> +			outstr = "unknown error";
>  #else
> -				outstr = strerror(errno);
> +			outstr = strerror(errno);
>  #endif /* NOLIBC_IGNORE_ERRNO */
> -			} else {
> -				if (ch != '%') {
> -					/* Invalid format: back up to output the format characters */
> -					fmt = outstr + 1;
> -					/* and output a '%' now. */
> -				}
> -				/* %% is documented as a 'conversion specifier'.
> -				 * Any flags, precision or length modifier are ignored.
> -				 */
> -				width = 0;
> -				outstr = "%";
> -			}
> -			len = strlen(outstr);
> +			goto do_strlen_output;
>  		}
>  
> +		if (ch != '%') {
> +			/* Invalid format: back up to output the format characters */
> +			fmt = outstr + 1;
> +			/* and output a '%' now. */
> +		}
> +		/* %% is documented as a 'conversion specifier'.
> +		 * Any flags, precision or length modifier are ignored.
> +		 */
> +		len = 1;
> +		width = 0;
> +		outstr = fmt - 1;
> +		goto do_output;
> +
> +do_strlen_output:
> +		/* Open coded strlen() (slightly smaller). */
> +		for (len = 0;; len++)
> +			if (!outstr[len])
> +				break;
> +
> +do_output:
>  		written += len;
>  
>  		width -= len;
> -- 
> 2.39.5