From: David Laight <david.laight.linux@gmail.com>

Use a single 'flags' variable to hold both format flags and length modifiers.
Use (1u << (c & 31)) for the flag bits to reduce code complexity.

Add support for left justifying fields.

Add support for length modifiers 't' and 'z' (both long) and 'q' and 'L'
(both long long).

Unconditionall generate the signed values (for %d) to remove a second
set of checks for the size.

Use 'signed int' for the lengths to make the pad test simpler.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 tools/include/nolibc/stdio.h | 88 ++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 164d2384978e..1ce4d357a802 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -240,20 +240,20 @@ char *fgets(char *s, int size, FILE *stream)
 }
 
 
-/* minimal printf(). It supports the following formats:
- *  - %[l*]{d,u,c,x,p}
- *  - %s
- *  - unknown modifiers are ignored.
+/* simple printf(). It supports the following formats:
+ *  - %[-][width][{l,t,z,ll,L,j,q}]{d,u,c,x,p,s,m}
+ *  - %%
+ *  - invalid formats are copied to the output buffer
  */
 typedef int (*__nolibc_printf_cb)(void *state, const char *buf, size_t size);
 
+#define __PF_FLAG(c) (1u << ((c) & 0x1f))
 static __attribute__((unused, format(printf, 3, 0)))
 int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list args)
 {
-	char lpref, c;
-	unsigned long long v;
-	unsigned int written, width;
-	size_t len;
+	char c;
+	int len, written, width;
+	unsigned int flags;
 	char tmpbuf[21];
 	const char *outstr;
 
@@ -265,6 +265,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 			break;
 
 		width = 0;
+		flags = 0;
 		if (c != '%') {
 			while (*fmt && *fmt != '%')
 				fmt++;
@@ -274,6 +275,13 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 
 			c = *fmt++;
 
+			/* Flag characters */
+			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
+				if ((__PF_FLAG(c) & (__PF_FLAG('-'))) == 0)
+					break;
+				flags |= __PF_FLAG(c);
+			}
+
 			/* width */
 			while (c >= '0' && c <= '9') {
 				width *= 10;
@@ -282,41 +290,34 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 				c = *fmt++;
 			}
 
-			/* Length modifiers */
-			if (c == 'l') {
-				lpref = 1;
-				c = *fmt++;
-				if (c == 'l') {
-					lpref = 2;
+			/* Length modifiers are lower case except 'L' which is the same a 'q' */
+			if ((c >= 'a' && c <= 'z') || (c == 'L' && (c = 'q'))) {
+				if (__PF_FLAG(c) & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z') |
+						    __PF_FLAG('j') | __PF_FLAG('q'))) {
+					if (c == 'l' && fmt[0] == 'l') {
+						fmt++;
+						c = 'q';
+					}
+					/* These all miss "# -0+" */
+					flags |= __PF_FLAG(c);
 					c = *fmt++;
 				}
-			} else if (c == 'j') {
-				/* intmax_t is long long */
-				lpref = 2;
-				c = *fmt++;
-			} else {
-				lpref = 0;
 			}
 
 			if (c == 'c' || c == 'd' || c == 'u' || c == 'x' || c == 'p') {
+				unsigned long long v;
+				long long signed_v;
 				char *out = tmpbuf;
 
-				if (c == 'p')
+				if ((c == 'p') || (flags & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z')))) {
 					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
+					signed_v = (long)v;
+				} else if (flags & (__PF_FLAG('j') | __PF_FLAG('q'))) {
+					v = va_arg(args, unsigned long long);
+					signed_v = v;
+				} else {
 					v = va_arg(args, unsigned int);
-
-				if (c == 'd') {
-					/* sign-extend the value */
-					if (lpref == 0)
-						v = (long long)(int)v;
-					else if (lpref == 1)
-						v = (long long)(long)v;
+					signed_v = (int)v;
 				}
 
 				switch (c) {
@@ -325,7 +326,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 					out[1] = 0;
 					break;
 				case 'd':
-					i64toa_r(v, out);
+					i64toa_r(signed_v, out);
 					break;
 				case 'u':
 					u64toa_r(v, out);
@@ -366,14 +367,24 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 
 		written += len;
 
-		while (width > len) {
-			unsigned int pad_len = ((width - len - 1) & 15) + 1;
+                /* An OPTIMIZER_HIDE_VAR() seems to stop gcc back-merging this
+                 * code into one of the conditionals above.
+		 */
+                __asm__ volatile("" : "=r"(len) : "0"(len));
+
+		/* Output 'left pad', 'value' then 'right pad'. */
+		flags &= __PF_FLAG('-');
+		width -= len;
+		if (flags && cb(state, outstr, len) != 0)
+			return -1;
+		while (width > 0) {
+			int pad_len = ((width - 1) & 15) + 1;
 			width -= pad_len;
 			written += pad_len;
 			if (cb(state, "                ", pad_len) != 0)
 				return -1;
 		}
-		if (cb(state, outstr, len) != 0)
+		if (!flags && cb(state, outstr, len) != 0)
 			return -1;
 	}
 
@@ -382,6 +393,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 
 	return written;
 }
+#undef _PF_FLAG
 
 struct __nolibc_fprintf_cb_state {
 	FILE *stream;
-- 
2.39.5

Hi David,

On Tue, Feb 03, 2026 at 10:29:54AM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> Use a single 'flags' variable to hold both format flags and length modifiers.
> Use (1u << (c & 31)) for the flag bits to reduce code complexity.
> 
> Add support for left justifying fields.
> 
> Add support for length modifiers 't' and 'z' (both long) and 'q' and 'L'
> (both long long).
> 
> Unconditionall generate the signed values (for %d) to remove a second
> set of checks for the size.
> 
> Use 'signed int' for the lengths to make the pad test simpler.
> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>  tools/include/nolibc/stdio.h | 88 ++++++++++++++++++++----------------
>  1 file changed, 50 insertions(+), 38 deletions(-)
> 
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index 164d2384978e..1ce4d357a802 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
> @@ -240,20 +240,20 @@ char *fgets(char *s, int size, FILE *stream)
>  }
>  
>  
> -/* minimal printf(). It supports the following formats:
> - *  - %[l*]{d,u,c,x,p}
> - *  - %s
> - *  - unknown modifiers are ignored.
> +/* simple printf(). It supports the following formats:
> + *  - %[-][width][{l,t,z,ll,L,j,q}]{d,u,c,x,p,s,m}
> + *  - %%
> + *  - invalid formats are copied to the output buffer
>   */
>  typedef int (*__nolibc_printf_cb)(void *state, const char *buf, size_t size);
>  
> +#define __PF_FLAG(c) (1u << ((c) & 0x1f))

This flag will be exposed to user code, you'll have to previx it with
_NOLIBC_.

>  static __attribute__((unused, format(printf, 3, 0)))
>  int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list args)
>  {
> -	char lpref, c;
> -	unsigned long long v;
> -	unsigned int written, width;
> -	size_t len;
> +	char c;
> +	int len, written, width;
> +	unsigned int flags;
>  	char tmpbuf[21];
>  	const char *outstr;
>  
> @@ -265,6 +265,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  			break;
>  
>  		width = 0;
> +		flags = 0;
>  		if (c != '%') {
>  			while (*fmt && *fmt != '%')
>  				fmt++;
> @@ -274,6 +275,13 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  
>  			c = *fmt++;
>  
> +			/* Flag characters */
> +			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
> +				if ((__PF_FLAG(c) & (__PF_FLAG('-'))) == 0)
> +					break;
> +				flags |= __PF_FLAG(c);
> +			}

Honestly I don't find that it improves readability here and makes one
keep doubts in background as "what if c == 'm' which will match '-'?".
I think that one would better be written as the usual:

			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
				if (c == '-')
					break;
				flags |= __PF_FLAG(c);
			}

Or even simpler since there's already a condition in the for() loop:

			for (; c >= 0x20 && c != '-' && c <= 0x3f; c = *fmt++)
				flags |= __PF_FLAG(c);

>  			/* width */
>  			while (c >= '0' && c <= '9') {
>  				width *= 10;
> @@ -282,41 +290,34 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  				c = *fmt++;
>  			}
>  
> -			/* Length modifiers */
> -			if (c == 'l') {
> -				lpref = 1;
> -				c = *fmt++;
> -				if (c == 'l') {
> -					lpref = 2;
> +			/* Length modifiers are lower case except 'L' which is the same a 'q' */
> +			if ((c >= 'a' && c <= 'z') || (c == 'L' && (c = 'q'))) {

Then please say "... which we replace by 'q'" so that we don't first read
that (c = 'q') as a possible typo. Also maybe add a mention to the fact
that "flags" will exclusively represent lowercase modifiers from now on?

> +				if (__PF_FLAG(c) & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z') |
> +						    __PF_FLAG('j') | __PF_FLAG('q'))) {

Even though I understand the value in checking bit positions (I use that
all the time as well), above this is just unreadable. Maybe you need a
different macro, maybe define another macro _NOLIBC_PF_LEN_MOD made of
the addition of all the flags to test against, I don't know, but the
construct, the line break in the middle of the expression and the
parenthesis needed for the macro just requires a lot of effort to
understand what's being tested. Alternately another possibility would
be to have another macro taking 4-5 char args and composing the flags
in one call, passing 0 or -1 for unused ones. This would also make
several parenthesis disappear which would help.

> +					if (c == 'l' && fmt[0] == 'l') {
> +						fmt++;
> +						c = 'q';
> +					}
> +					/* These all miss "# -0+" */
> +					flags |= __PF_FLAG(c);
>  					c = *fmt++;
>  				}
> -			} else if (c == 'j') {
> -				/* intmax_t is long long */
> -				lpref = 2;
> -				c = *fmt++;
> -			} else {
> -				lpref = 0;
>  			}
>  
>  			if (c == 'c' || c == 'd' || c == 'u' || c == 'x' || c == 'p') {
> +				unsigned long long v;
> +				long long signed_v;
>  				char *out = tmpbuf;
>  
> -				if (c == 'p')
> +				if ((c == 'p') || (flags & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z')))) {
>  					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
> +					signed_v = (long)v;
> +				} else if (flags & (__PF_FLAG('j') | __PF_FLAG('q'))) {
> +					v = va_arg(args, unsigned long long);
> +					signed_v = v;
> +				} else {
>  					v = va_arg(args, unsigned int);
> -
> -				if (c == 'd') {
> -					/* sign-extend the value */
> -					if (lpref == 0)
> -						v = (long long)(int)v;
> -					else if (lpref == 1)
> -						v = (long long)(long)v;
> +					signed_v = (int)v;
>  				}
>  
>  				switch (c) {
> @@ -325,7 +326,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  					out[1] = 0;
>  					break;
>  				case 'd':
> -					i64toa_r(v, out);
> +					i64toa_r(signed_v, out);
>  					break;
>  				case 'u':
>  					u64toa_r(v, out);
> @@ -366,14 +367,24 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  
>  		written += len;
>  
> -		while (width > len) {
> -			unsigned int pad_len = ((width - len - 1) & 15) + 1;
> +                /* An OPTIMIZER_HIDE_VAR() seems to stop gcc back-merging this
> +                 * code into one of the conditionals above.

Be careful, space indentation above.

> +		 */
> +                __asm__ volatile("" : "=r"(len) : "0"(len));

and here.

> +
> +		/* Output 'left pad', 'value' then 'right pad'. */
> +		flags &= __PF_FLAG('-');
> +		width -= len;
> +		if (flags && cb(state, outstr, len) != 0)
> +			return -1;
> +		while (width > 0) {
> +			int pad_len = ((width - 1) & 15) + 1;
>  			width -= pad_len;
>  			written += pad_len;
>  			if (cb(state, "                ", pad_len) != 0)
>  				return -1;
>  		}
> -		if (cb(state, outstr, len) != 0)
> +		if (!flags && cb(state, outstr, len) != 0)
>  			return -1;
>  	}
>  
> @@ -382,6 +393,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  
>  	return written;
>  }
> +#undef _PF_FLAG

This one can be dropped once named as _NOLIBC_xxx

Willy

On Wed, 4 Feb 2026 05:14:51 +0100
Willy Tarreau <w@1wt.eu> wrote:

> Hi David,
> 
> On Tue, Feb 03, 2026 at 10:29:54AM +0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > Use a single 'flags' variable to hold both format flags and length modifiers.
> > Use (1u << (c & 31)) for the flag bits to reduce code complexity.
> > 
> > Add support for left justifying fields.
> > 
> > Add support for length modifiers 't' and 'z' (both long) and 'q' and 'L'
> > (both long long).
> > 
> > Unconditionall generate the signed values (for %d) to remove a second
> > set of checks for the size.
> > 
> > Use 'signed int' for the lengths to make the pad test simpler.
> > 
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> >  tools/include/nolibc/stdio.h | 88 ++++++++++++++++++++----------------
> >  1 file changed, 50 insertions(+), 38 deletions(-)
> > 
> > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > index 164d2384978e..1ce4d357a802 100644
> > --- a/tools/include/nolibc/stdio.h
> > +++ b/tools/include/nolibc/stdio.h
> > @@ -240,20 +240,20 @@ char *fgets(char *s, int size, FILE *stream)
> >  }
> >  
> >  
> > -/* minimal printf(). It supports the following formats:
> > - *  - %[l*]{d,u,c,x,p}
> > - *  - %s
> > - *  - unknown modifiers are ignored.
> > +/* simple printf(). It supports the following formats:
> > + *  - %[-][width][{l,t,z,ll,L,j,q}]{d,u,c,x,p,s,m}
> > + *  - %%
> > + *  - invalid formats are copied to the output buffer
> >   */
> >  typedef int (*__nolibc_printf_cb)(void *state, const char *buf, size_t size);
> >  
> > +#define __PF_FLAG(c) (1u << ((c) & 0x1f))  
> 
> This flag will be exposed to user code, you'll have to previx it with
> _NOLIBC_.

The lines are long enough already, something shorter would be ideal.
The #undef at the bottom stops it being exposed.

> 
> >  static __attribute__((unused, format(printf, 3, 0)))
> >  int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list args)
> >  {
> > -	char lpref, c;
> > -	unsigned long long v;
> > -	unsigned int written, width;
> > -	size_t len;
> > +	char c;
> > +	int len, written, width;
> > +	unsigned int flags;
> >  	char tmpbuf[21];
> >  	const char *outstr;
> >  
> > @@ -265,6 +265,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  			break;
> >  
> >  		width = 0;
> > +		flags = 0;
> >  		if (c != '%') {
> >  			while (*fmt && *fmt != '%')
> >  				fmt++;
> > @@ -274,6 +275,13 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  
> >  			c = *fmt++;
> >  
> > +			/* Flag characters */
> > +			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
> > +				if ((__PF_FLAG(c) & (__PF_FLAG('-'))) == 0)
> > +					break;
> > +				flags |= __PF_FLAG(c);
> > +			}  
> 
> Honestly I don't find that it improves readability here and makes one
> keep doubts in background as "what if c == 'm' which will match '-'?".
> I think that one would better be written as the usual:
> 
> 			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
> 				if (c == '-')
> 					break;
> 				flags |= __PF_FLAG(c);
> 			}
> 
> Or even simpler since there's already a condition in the for() loop:
> 
> 			for (; c >= 0x20 && c != '-' && c <= 0x3f; c = *fmt++)
> 				flags |= __PF_FLAG(c);

It is all written that way for when more flags get added - look at the later
patches which check for any of "#-+ 0".
At that point the line get long and unreadable - as below :-)

I didn't want to add the flags here before supporting them later.
But they could all be accepted and ignored until implemented.
That might be better anyway.

Actually it might be worth s/c/ch/ to make the brain see the difference
between c and 'c' more easily.

Perhaps I'm expand the comment a bit.
It is all a hint as to what is happening later on with the character tests.

> 
> >  			/* width */
> >  			while (c >= '0' && c <= '9') {
> >  				width *= 10;
> > @@ -282,41 +290,34 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  				c = *fmt++;
> >  			}
> >  
> > -			/* Length modifiers */
> > -			if (c == 'l') {
> > -				lpref = 1;
> > -				c = *fmt++;
> > -				if (c == 'l') {
> > -					lpref = 2;
> > +			/* Length modifiers are lower case except 'L' which is the same a 'q' */
> > +			if ((c >= 'a' && c <= 'z') || (c == 'L' && (c = 'q'))) {  
> 
> Then please say "... which we replace by 'q'" so that we don't first read
> that (c = 'q') as a possible typo. Also maybe add a mention to the fact
> that "flags" will exclusively represent lowercase modifiers from now on?

I'll clarify it.

> 
> > +				if (__PF_FLAG(c) & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z') |
> > +						    __PF_FLAG('j') | __PF_FLAG('q'))) {  
> 
> Even though I understand the value in checking bit positions (I use that
> all the time as well), above this is just unreadable. Maybe you need a
> different macro, maybe define another macro _NOLIBC_PF_LEN_MOD made of
> the addition of all the flags to test against, I don't know, but the
> construct, the line break in the middle of the expression and the
> parenthesis needed for the macro just requires a lot of effort to
> understand what's being tested. Alternately another possibility would
> be to have another macro taking 4-5 char args and composing the flags
> in one call, passing 0 or -1 for unused ones. This would also make
> several parenthesis disappear which would help.

Hmmm, some macro magic might work, loosely:
#define FLNZ(q) (q ? 1 << (q & 31) ? 0)
#define FLM3(q1, q2, q3, ...) ((FLNZ(q1) | FLNZ(q2) | FLNZ(q3))
#define FLT(fl, ...) (fl & FLM3(__VA_ARGS__, 0, 0, 0))
#define CT(c, ...) FLT(1 << (c & 31), __VA_ARGS__) 
Then the above would be:
	if (CT(c, 'l', 't', 'z', 'j', 'q')) {
Clearly needs some better and longer names and a big comment block.

> 
> > +					if (c == 'l' && fmt[0] == 'l') {
> > +						fmt++;
> > +						c = 'q';
> > +					}
> > +					/* These all miss "# -0+" */
> > +					flags |= __PF_FLAG(c);
> >  					c = *fmt++;
> >  				}
> > -			} else if (c == 'j') {
> > -				/* intmax_t is long long */
> > -				lpref = 2;
> > -				c = *fmt++;
> > -			} else {
> > -				lpref = 0;
> >  			}
> >  
> >  			if (c == 'c' || c == 'd' || c == 'u' || c == 'x' || c == 'p') {
> > +				unsigned long long v;
> > +				long long signed_v;
> >  				char *out = tmpbuf;
> >  
> > -				if (c == 'p')
> > +				if ((c == 'p') || (flags & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z')))) {
> >  					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
> > +					signed_v = (long)v;
> > +				} else if (flags & (__PF_FLAG('j') | __PF_FLAG('q'))) {
> > +					v = va_arg(args, unsigned long long);
> > +					signed_v = v;
> > +				} else {
> >  					v = va_arg(args, unsigned int);
> > -
> > -				if (c == 'd') {
> > -					/* sign-extend the value */
> > -					if (lpref == 0)
> > -						v = (long long)(int)v;
> > -					else if (lpref == 1)
> > -						v = (long long)(long)v;
> > +					signed_v = (int)v;
> >  				}
> >  
> >  				switch (c) {
> > @@ -325,7 +326,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  					out[1] = 0;
> >  					break;
> >  				case 'd':
> > -					i64toa_r(v, out);
> > +					i64toa_r(signed_v, out);
> >  					break;
> >  				case 'u':
> >  					u64toa_r(v, out);
> > @@ -366,14 +367,24 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  
> >  		written += len;
> >  
> > -		while (width > len) {
> > -			unsigned int pad_len = ((width - len - 1) & 15) + 1;
> > +                /* An OPTIMIZER_HIDE_VAR() seems to stop gcc back-merging this
> > +                 * code into one of the conditionals above.  
> 
> Be careful, space indentation above.
> 
> > +		 */
> > +                __asm__ volatile("" : "=r"(len) : "0"(len));  
> 
> and here.

Oops...

> 
> > +
> > +		/* Output 'left pad', 'value' then 'right pad'. */
> > +		flags &= __PF_FLAG('-');
> > +		width -= len;
> > +		if (flags && cb(state, outstr, len) != 0)
> > +			return -1;
> > +		while (width > 0) {
> > +			int pad_len = ((width - 1) & 15) + 1;
> >  			width -= pad_len;
> >  			written += pad_len;
> >  			if (cb(state, "                ", pad_len) != 0)
> >  				return -1;
> >  		}
> > -		if (cb(state, outstr, len) != 0)
> > +		if (!flags && cb(state, outstr, len) != 0)
> >  			return -1;
> >  	}
> >  
> > @@ -382,6 +393,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  
> >  	return written;
> >  }
> > +#undef _PF_FLAG  
> 
> This one can be dropped once named as _NOLIBC_xxx

I'll see if I can get a max of 2 expansions on a line.
Otherwise the lines get horribly long.

	David

> 
> Willy

On Wed, Feb 04, 2026 at 10:17:05AM +0000, David Laight wrote:
> > This flag will be exposed to user code, you'll have to previx it with
> > _NOLIBC_.
> 
> The lines are long enough already, something shorter would be ideal.
> The #undef at the bottom stops it being exposed.

No, it's not just about not being exposed, it's about *conflicting*.
We just do not reserve macro names not starting with anything but
_NOLIBC. If I already use __PF_BASE in my application, it will cause
trouble here.

> > > +			/* Flag characters */
> > > +			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
> > > +				if ((__PF_FLAG(c) & (__PF_FLAG('-'))) == 0)
> > > +					break;
> > > +				flags |= __PF_FLAG(c);
> > > +			}  
> > 
> > Honestly I don't find that it improves readability here and makes one
> > keep doubts in background as "what if c == 'm' which will match '-'?".
> > I think that one would better be written as the usual:
> > 
> > 			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
> > 				if (c == '-')
> > 					break;
> > 				flags |= __PF_FLAG(c);
> > 			}
> > 
> > Or even simpler since there's already a condition in the for() loop:
> > 
> > 			for (; c >= 0x20 && c != '-' && c <= 0x3f; c = *fmt++)
> > 				flags |= __PF_FLAG(c);
> 
> It is all written that way for when more flags get added - look at the later
> patches which check for any of "#-+ 0".
> At that point the line get long and unreadable - as below :-)

I know, I've seen them and am already bothered by this. The
purpose of that lib has always been to focus on:
  1) size
  2) maintainability
  3) portability

Performance has never been a concern. I totally agree that testing
bitfields is often much shorter than multiple "if", though here I'm
seeing the code significantly inflate with loops etc (which might
remain small), but maintainability is progressively reducing. This
code receives few contributions from many participants, and it's
important that it's easy to understand what's being done in order
to easily add your missing feature. I'm feeling that we're starting
to steer away from this principle here, which is why I'm raising an
alarm.

> I didn't want to add the flags here before supporting them later.
> But they could all be accepted and ignored until implemented.
> That might be better anyway.

I've seen that in a later patch you have up to 10 values tested in
chain. I just think that it could be sufficient to have a macro
taking 10 char args, that remains easy enough to use and understand
where it is, e.g. you pass the base then all values:

    _NOLIBC_ANY_OF(0x20, 'c', 'd', 'r', 'z', -1, -1, -1, -1 ...)

> Actually it might be worth s/c/ch/ to make the brain see the difference
> between c and 'c' more easily.

I'm not sure it's the only detail which is complexifying my reading :-/

> Perhaps I'm expand the comment a bit.

Yes, comments are cheap and welcoming to new readers.

> It is all a hint as to what is happening later on with the character tests.

I roughly get what you're trying to do and am not contesting the goals,
I'm however questioning the size efficiency of the resulting code (not
fully certain it remains as small as reasonably possible), and the
ease of maintenance.

> > 
> > > +				if (__PF_FLAG(c) & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z') |
> > > +						    __PF_FLAG('j') | __PF_FLAG('q'))) {  
> > 
> > Even though I understand the value in checking bit positions (I use that
> > all the time as well), above this is just unreadable. Maybe you need a
> > different macro, maybe define another macro _NOLIBC_PF_LEN_MOD made of
> > the addition of all the flags to test against, I don't know, but the
> > construct, the line break in the middle of the expression and the
> > parenthesis needed for the macro just requires a lot of effort to
> > understand what's being tested. Alternately another possibility would
> > be to have another macro taking 4-5 char args and composing the flags
> > in one call, passing 0 or -1 for unused ones. This would also make
> > several parenthesis disappear which would help.
> 
> Hmmm, some macro magic might work, loosely:
> #define FLNZ(q) (q ? 1 << (q & 31) ? 0)
> #define FLM3(q1, q2, q3, ...) ((FLNZ(q1) | FLNZ(q2) | FLNZ(q3))
> #define FLT(fl, ...) (fl & FLM3(__VA_ARGS__, 0, 0, 0))
> #define CT(c, ...) FLT(1 << (c & 31), __VA_ARGS__) 
> Then the above would be:
> 	if (CT(c, 'l', 't', 'z', 'j', 'q')) {
> Clearly needs some better and longer names and a big comment block.

Yes maybe something like this (with _NOLIBC_ please).

> > > +#undef _PF_FLAG  
> > 
> > This one can be dropped once named as _NOLIBC_xxx
> 
> I'll see if I can get a max of 2 expansions on a line.
> Otherwise the lines get horribly long.

OK but with todays screens it's less of a problem, and often early
wrapping affects legibility more than long lines :-/  We don't have
a strict 80-char limit in this project, so if you need 100 to make
something more readable once in a while, please just do it.

But please also keep in mind my comments about the goals of size,
maintainability and portability (e.g. don't forget to compare
size before/after, and at least to mention when some significant
changes have impacts in one direction or the other because that
matters).

Thanks,
Willy

On Wed, 4 Feb 2026 11:40:27 +0100
Willy Tarreau <w@1wt.eu> wrote:

> On Wed, Feb 04, 2026 at 10:17:05AM +0000, David Laight wrote:
> > > This flag will be exposed to user code, you'll have to previx it with
> > > _NOLIBC_.  
> > 
> > The lines are long enough already, something shorter would be ideal.
> > The #undef at the bottom stops it being exposed.  
> 
> No, it's not just about not being exposed, it's about *conflicting*.
> We just do not reserve macro names not starting with anything but
> _NOLIBC. If I already use __PF_BASE in my application, it will cause
> trouble here.

I'd also #undef'ed the wrong name.

> 
> > > > +			/* Flag characters */
> > > > +			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
> > > > +				if ((__PF_FLAG(c) & (__PF_FLAG('-'))) == 0)
> > > > +					break;
> > > > +				flags |= __PF_FLAG(c);
> > > > +			}    
> > > 
> > > Honestly I don't find that it improves readability here and makes one
> > > keep doubts in background as "what if c == 'm' which will match '-'?".
> > > I think that one would better be written as the usual:
> > > 
> > > 			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
> > > 				if (c == '-')
> > > 					break;
> > > 				flags |= __PF_FLAG(c);
> > > 			}
> > > 
> > > Or even simpler since there's already a condition in the for() loop:
> > > 
> > > 			for (; c >= 0x20 && c != '-' && c <= 0x3f; c = *fmt++)
> > > 				flags |= __PF_FLAG(c);  
> > 
> > It is all written that way for when more flags get added - look at the later
> > patches which check for any of "#-+ 0".
> > At that point the line get long and unreadable - as below :-)  
> 
> I know, I've seen them and am already bothered by this. The
> purpose of that lib has always been to focus on:
>   1) size
>   2) maintainability
>   3) portability
> 
> Performance has never been a concern. I totally agree that testing
> bitfields is often much shorter than multiple "if", though here I'm
> seeing the code significantly inflate with loops etc (which might
> remain small), but maintainability is progressively reducing. This
> code receives few contributions from many participants, and it's
> important that it's easy to understand what's being done in order
> to easily add your missing feature. I'm feeling that we're starting
> to steer away from this principle here, which is why I'm raising an
> alarm.

I've done some changes - I'll need to back-merge them into the patches.
That code now looks like:

			/* Conversion flag characters are all non-alphabetic.
			 * Create a bit-map of the valid ones for later.
			 */
			for (; ch >= 0x20 && ch <= 0x3f; ch = *fmt++) {
				if (!_NOLIBC_PF_CHAR_IS_ONE_OF(ch, ' ', '#', '+', '-', '0'))
					break;
				flags |= _NOLIBC_PF_FLAG(ch);
			}

Which is definitely more readable (until you look inside the #define).
I could even hide the range check inside the macro by using the
high bits of the first character.
That would then read:
	while (_NOLIBC_PF_CHAR_IS_ONE_OF(ch, ' ', '#', '+', '-', '0')) {
		flags |= _NOLIBC_PF_FLAG(ch);
		ch = *fmt++;
	}
or maybe:
	for (fl = 1; fl; ch = &fmt++) {
		fl = _NOLIBC_PF_CHAR_IS_ONE_OF(ch, ' ', '#', '+', '-', '0');
		flags |= fl;
	}
(especially if gcc generates a lot less code for it - I expect it will
pessimise it back to the other version.)

> 
> > I didn't want to add the flags here before supporting them later.
> > But they could all be accepted and ignored until implemented.
> > That might be better anyway.  
> 
> I've seen that in a later patch you have up to 10 values tested in
> chain.

I only counted 7 :-)
I used the __VA_ARGS__, 0, 0 trick to support up to 8.

> I just think that it could be sufficient to have a macro
> taking 10 char args, that remains easy enough to use and understand
> where it is, e.g. you pass the base then all values:
> 
>     _NOLIBC_ANY_OF(0x20, 'c', 'd', 'r', 'z', -1, -1, -1, -1 ...)
> 
> > Actually it might be worth s/c/ch/ to make the brain see the difference
> > between c and 'c' more easily.  
> 
> I'm not sure it's the only detail which is complexifying my reading :-/

It is a simple one that does make a difference.
It would have to be the first patch, which will make 'rebasing' the
patches a nightmare.

> 
> > Perhaps I'm expand the comment a bit.  
> 
> Yes, comments are cheap and welcoming to new readers.
> 
> > It is all a hint as to what is happening later on with the character tests.  
> 
> I roughly get what you're trying to do and am not contesting the goals,
> I'm however questioning the size efficiency of the resulting code (not
> fully certain it remains as small as reasonably possible), and the
> ease of maintenance.

I've been running the bloat-o-meter on pretty much every build.
The current build of __nolibc_printf (without my changes to stdlib.h) is at +90 bytes.
But I think it has lost an inlined copy of u64toa_r (about 144 bytes).
Actual size is about 1k.

The bit-flags variables definitely help over multiple comparisons.
The width/precision loop helps due to the size of va_arg()
(that is one of the bits that actually increases the size).

The code to buffer printf() (to file) actually adds more than the other changes.
But I like it because of the difference it makes to strace.

Don't even look at the mess gcc creates for the obvious:
	precision = width - (size != 0) - (size > 255);

And I can't get it to use 'bt' (bit test) or 'bts' (bit test and set).
Code like:
	if (_NOLIBC_PF_CHAR_IS_ONE_OF(ch, ' ', '#', '+', '-', '0'))
		flags |= _NOLIBC_PF_FLAG(ch);
Could be:
	mask = 0x....;
	if (bt(ch, mask))
		bts(ch, flags);
But it insists on doing:
	if ((mask >> ch) & 1)
		flags |= 1 << ch;
I can't even get it to do:
	if (mask & (1 << ch))
		flags |= 1 << ch;
(I need the condition for loop control.)
I've not tried variants of:
	bit = mask & (1 << ch);
	flags |= bit;
	if (bit)...

There is on 'bts' - in the code that adds the '#' flag into the conversion
character. Doing that improves the code far more than you might expect.
To the point where adding |= 1 << 'b' might actually reduce the code size
even though 'b' is never tested for.

...
> > I'll see if I can get a max of 2 expansions on a line.
> > Otherwise the lines get horribly long.  
> 
> OK but with todays screens it's less of a problem, and often early
> wrapping affects legibility more than long lines :-/  We don't have
> a strict 80-char limit in this project, so if you need 100 to make
> something more readable once in a while, please just do it.

I meant 'really horrible' ...
A few of the lines are in the 90s, one might be 103.
I still like 80 characters, but anal continuation lines are silly.
(I found some (ex)dayjob code written 20+ years ago with 200+
character lines, no idea how they edited it.)

> 
> But please also keep in mind my comments about the goals of size,
> maintainability and portability (e.g. don't forget to compare
> size before/after, and at least to mention when some significant
> changes have impacts in one direction or the other because that
> matters).

I have been looking at size.
I ripped some changes out because they didn't make things smaller
and made them very much less readable.

It when down quite a bit when I did the restructure and then crept
back up as I added extra features.
Overall there isn't that much difference.

One thing that would reduce overall size is making the non-trivial
functions in stdlib.h (and maybe elsewhere) noinline.
(I set that for some of my builds to stop the numeric convertion
functions being inlined - no idea why they aren't considered too big.)

For gcc adding noclone may also help (not supported by clang).
noclone can have a strange effect.
I set it on my u64toa_base() functions but only enabled it for decimal.
There were two call sites, both passed in the two constants.
But the called function ignored the passed values and reloaded both constants.
Not at all 'what the crowd intended' :-)

	David

> 
> Thanks,
> Willy