linux-kernel - Re: [PATCH v3 1/2] tools/nolibc: support left alignment (-) in printf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260130081435.632f5fe9@pumpkin>
Date: Fri, 30 Jan 2026 08:14:35 +0000
From: David Laight <david.laight.linux@...il.com>
To: "licheng.li" <im.lechain@...il.com>
Cc: Willy Tarreau <w@....eu>, Thomas Weißschuh
 <linux@...ssschuh.net>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] tools/nolibc: support left alignment (-) in
 printf

On Fri, 30 Jan 2026 15:18:46 +0800
"licheng.li" <im.lechain@...il.com> wrote:

> From: Cheng Li <im.lechain@...il.com>
> 
> Currently, __nolibc_printf() in nolibc parses the width field but always
> pads with spaces on the left. It ignores the '-' flag (left alignment).
> 
> This patch implements support for the '-' flag
> to forces left alignment by padding spaces on the right.
> 
> The implementation reuses the padding character logic to handle both
> cases.

You don't have two cases any more.

> Logic behavior:
>  - "%5d"  -> "   -5" (unchanged, right align)
>  - "%-5d" -> "-5   " (new, left align)
> 
> Suggested-by: Willy Tarreau <w@....eu>
> Signed-off-by: Cheng Li <im.lechain@...il.com>
> ---
> v3 changes:
>  - Removed pad zeros support because of bug for signed number and pointer
> v2 changes:
>  - Adopted optimization suggestions from Willy Tarreau:
>    - Incremented 'written' counter at the start of loops.
>    - Reordered loop checks to optimize compiler register usage.
>  - Updated commit message to explicitly mention zero-padding ('0') support.
> ---
>  tools/include/nolibc/stdio.h | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index 1f16dab2ac88..f31b77f61d3b 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
> @@ -250,7 +250,7 @@ typedef int (*__nolibc_printf_cb)(intptr_t state, const char *buf, size_t size);
>  static __attribute__((unused, format(printf, 4, 0)))
>  int __nolibc_printf(__nolibc_printf_cb cb, intptr_t state, size_t n, const char *fmt, va_list args)
>  {
> -	char escape, lpref, c;
> +	char escape, lpref, padc, c;
>  	unsigned long long v;
>  	unsigned int written, width;
>  	size_t len, ofs, w;
> @@ -261,11 +261,17 @@ int __nolibc_printf(__nolibc_printf_cb cb, intptr_t state, size_t n, const char
>  	while (1) {
>  		c = fmt[ofs++];
>  		width = 0;
> +		padc = ' ';
>  
>  		if (escape) {
>  			/* we're in an escape sequence, ofs == 1 */
>  			escape = 0;
>  
> +			if (c == '-') {
> +				padc = c;
> +				c = fmt[ofs++];
> +			}

You only need a 'left align' flag.

> +
>  			/* width */
>  			while (c >= '0' && c <= '9') {
>  				width *= 10;
> @@ -358,13 +364,20 @@ int __nolibc_printf(__nolibc_printf_cb cb, intptr_t state, size_t n, const char
>  			if (n) {
>  				w = len < n ? len : n;
>  				n -= w;
> -				while (width-- > w) {
> +				while (width > w && padc != '-') {
> +					written += 1;
>  					if (cb(state, " ", 1) != 0)
>  						return -1;
> -					written += 1;
> +					width--;
>  				}
>  				if (cb(state, outstr, w) != 0)
>  					return -1;
> +				while (width > w) {
> +					written += 1;
> +					if (cb(state, " ", 1) != 0)
> +						return -1;
> +					width--;
> +				}

I'd swap the code so the short bit is replicated - should be smaller:
				if (padc == '-' && cb(state, outstr, w) != 0)
					return -1;
				while (width > w) {
					written += 1;
					if (cb(state, " ", 1) != 0)
						return -1;
					width--;
				}
				if (padc != '-' && cb(state, outstr, w) != 0)
					return -1;

There is also a slight inconsistency between 'width--' and 'written += 1'.
I'd use the postfix operator for both, but that is a matter of taste.

	David

>  			}
>  
>  			written += len;