Message-ID: <CAAZOWcKtMqsFZ91yJAek6J4bBecgRrECzBjuuG20ooU_ZrKayw@mail.gmail.com>
Date: Fri, 30 Jan 2026 09:05:39 +0800
From: Cheng Li <im.lechain@...il.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Willy Tarreau <w@....eu>, Thomas Weißschuh <linux@...ssschuh.net>, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] tools/nolibc: support left alignment (-) and zero
 padding (0) in printf

David Laight <david.laight.linux@...il.com> 于2026年1月29日周四 18:30写道：
>
> On Wed, 28 Jan 2026 17:42:23 +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)
> > 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".

You are absolutely right. I just tested printf("%05d", -5) and it
resulted in 000-5,
which is incorrect. Standard behavior should be -0005.

Since nolibc constructs the integer string (including the sign) before
this padding logic,
implementing correct zero-padding for negative numbers would require
parsing the buffer
or refactoring the conversion logic. This would add significant
complexity and size,
which goes against the goal of this small patch.

Proposed solution:
I will drop the implicit support for the '0' flag from this patch and
revert to my original goal: strictly supporting left alignment ('-').

Left alignment pads spaces on the right (-5 ), so it does not suffer
from the sign issue.

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

These are great suggestions for a larger refactoring of printf to
improve performance and standard compliance.
However, to keep this current change minimal and safe, I prefer to
focus on fixing the alignment feature first.

@Willy, I will send a v3 that removes the '0' flag handling logic to
avoid the bug pointed out above.

I will rethink how to implement leading zero pad support.

Best regards,
Cheng

> 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@....eu>
> > Signed-off-by: Cheng Li <im.lechain@...il.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;
>

Powered by blists - more mailing lists