All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: "licheng.li" <im.lechain@gmail.com>
Cc: "Willy Tarreau" <w@1wt.eu>,
	"Thomas Weißschuh" <linux@weissschuh.net>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] tools/nolibc: support left alignment (-) and zero padding (0) in printf
Date: Thu, 29 Jan 2026 10:30:57 +0000	[thread overview]
Message-ID: <20260129103057.324d654d@pumpkin> (raw)
In-Reply-To: <20260128094224.11299-2-im.lechain@gmail.com>

On Wed, 28 Jan 2026 17:42:23 +0800
"licheng.li" <im.lechain@gmail.com> wrote:

> From: Cheng Li <im.lechain@gmail.com>
> 
> Currently, __nolibc_printf() in nolibc parses the width field but always
> pads with spaces on the left. It ignores the '-' flag (left alignment)
> and treats leading zeros in the width as part of the number parsing
> (resulting in space padding instead of zero padding).
> 
> This patch implements support for:
> 1. The '-' flag: forces left alignment by padding spaces on the right.
> 2. The '0' flag: forces zero padding on the left instead of spaces.

Does that work properly?
I think it is padding zeros the wrong side of the sign (or "0x").
Try testing with a negative number or a pointer.

You might need to limit the width for numeric fields to (say) 32,
make the on-stack buffer 64 bytes and convert into the middle.
Then add pad zeros to the front before adding any "-" or "0x".

It then gets very close to supporting precisions as well as widths.
It might be worth changing the 'pad' loop to:
	while (width > w) {
		unsigned int pad_len = ((width - w - 1) & 0xf) + 1;
		width -= pad_len;
		written += pad_len;
		if (cb(state, "                ", pad_len) != 0)
			return -1;
	}
while the code is marginally larger writing the pad in blocks of 16
will be much faster.
The linker should merge all the strings of spaces.

I looked at the (existing) version last night.
If the 'l' flag code is moved to after the width is calculated it all
becomes a lot simpler.
Set 'outstr = fmt' at the top of the loop and the code can use *fmt++
and do a 'fast scan' of the format string for a '%' or '\0' then
output the chars from the format string at the bottom.
I didn't size the change, but getting rid of ofs and escape must help.

Might post it later.

	David


> 
> The implementation reuses the padding character logic to handle both
> cases.
> 
> Logic behavior:
>  - "%5d"  -> "   12" (unchanged)
>  - "%-5d" -> "12   " (new: left align)
>  - "%05d" -> "00012" (new: zero pad)
>  - "%-05d"-> "12   " (new: left align overrides zero pad)
> 
> The code is optimized to keep the binary size impact minimal.
> Measuring the nolibc-test binary on x86_64:
> 
>   text    data     bss     dec     hex filename
>  43552     248     112   43912    ab88 nolibc-test (before)
>  43677     248     112   44037    ac05 nolibc-test (after)
> 
> The net increase is 125 bytes.
> 
> Suggested-by: Willy Tarreau <w@1wt.eu>
> Signed-off-by: Cheng Li <im.lechain@gmail.com>
> ---
> 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 | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index 1f16dab2ac88..df8a96e39986 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 == '-' || c == '0') {
> +				padc = c;
> +				c = fmt[ofs++];
> +			}
> +
>  			/* width */
>  			while (c >= '0' && c <= '9') {
>  				width *= 10;
> @@ -358,13 +364,19 @@ 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) {
> -					if (cb(state, " ", 1) != 0)
> -						return -1;
> +				while (width > w && padc != '-') {
>  					written += 1;
> +					if (cb(state, &padc, 1) != 0)
> +						return -1;
> +					width--;
>  				}
>  				if (cb(state, outstr, w) != 0)
>  					return -1;
> +				while (width-- > w) {
> +					written += 1;
> +					if (cb(state, " ", 1) != 0)
> +						return -1;
> +				}
>  			}
>  
>  			written += len;


  parent reply	other threads:[~2026-01-29 10:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-28  9:42 [PATCH v2 0/2] tools/nolibc: printf left alignment and zero padding licheng.li
2026-01-28  9:42 ` [PATCH v2 1/2] tools/nolibc: support left alignment (-) and zero padding (0) in printf licheng.li
2026-01-29  6:41   ` Willy Tarreau
2026-01-29  6:57     ` licheng
2026-01-29  7:07       ` Willy Tarreau
2026-01-29 10:30   ` David Laight [this message]
2026-01-30  1:05     ` Cheng Li
2026-01-30  8:01       ` David Laight
2026-01-30  9:52         ` Cheng Li
2026-01-28  9:42 ` [PATCH v2 2/2] selftests/nolibc: add tests for printf left alignment and zero padding licheng.li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260129103057.324d654d@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=im.lechain@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.