public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] tools/nolibc/printf: Support negative variable width and precision
@ 2026-03-23 11:22 david.laight.linux
  2026-03-25 20:37 ` Thomas Weißschuh
  0 siblings, 1 reply; 3+ messages in thread
From: david.laight.linux @ 2026-03-23 11:22 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, linux-kernel; +Cc: David Laight

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

For (eg) "%*.*s" treat a negative field width as a request to left align
the output (the same as the '-' flag), and a negative precision to
request the default precision.

Set the default precision to -1 (not INT_MAX) and add explicit checks
to the string handling for negative values (makes the tet unsigned).

For numeric output check for 'precision >= 0' instead of testing
_NOLIBC_PF_FLAGS_CONTAIN(flags, '.').
This needs an inverted test, some extra goto and removes an indentation.
The changed conditionals fix printf("%0-#o", 0) - but '0' and '-' shouldn't
both be specified.

Best viewed with 'git diff -b' after being commited.

Additional test cases added.

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

I missed this bit in the earlier patches.
Size wise it is pretty neutral. It really seems to depend on how many registers
get saved across the call to _nolibc_u64toa_base() - gcc doesn't seem to use
the correct registers to avoid spills.

I did look at whether making 'width' negative at the top was better than
keeping a '-' flag - but it bloats things because you need the absolute value
at the bottom.

 tools/include/nolibc/stdio.h                 | 68 +++++++++++---------
 tools/testing/selftests/nolibc/nolibc-test.c |  5 +-
 2 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 8f7e1948a651..b6d14a58cfe7 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -347,6 +347,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 	char *out;
 	const char *outstr;
 	unsigned int sign_prefix;
+	int got_width;
 
 	written = 0;
 	while (1) {
@@ -377,23 +378,28 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 		}
 
 		/* Width and precision */
-		for (;; ch = *fmt++) {
+		for (got_width = 0;; ch = *fmt++) {
 			if (ch == '*') {
-				precision = va_arg(args, unsigned int);
+				precision = va_arg(args, int);
 				ch = *fmt++;
 			} else {
 				for (precision = 0; ch >= '0' && ch <= '9'; ch = *fmt++)
 					precision = precision * 10 + (ch - '0');
 			}
-			if (_NOLIBC_PF_FLAGS_CONTAIN(flags, '.'))
+			if (got_width)
 				break;
 			width = precision;
 			if (ch != '.') {
 				/* Default precision for strings */
-				precision = INT_MAX;
+				precision = -1;
 				break;
 			}
-			flags |= _NOLIBC_PF_FLAG('.');
+			got_width = 1;
+		}
+		/* A negative width (e.g. from "%*s") requests left justify. */
+		if (width < 0) {
+			width = -width;
+			flags |= _NOLIBC_PF_FLAG('-');
 		}
 
 		/* Length modifier.
@@ -457,7 +463,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 				if (!outstr) {
 					outstr = "(null)";
 					/* Match glibc, nothing output if precision too small */
-					len = precision >= 6 ? 6 : 0;
+					len = precision < 0 || precision >= 6 ? 6 : 0;
 					goto do_output;
 				}
 				goto do_strlen_output;
@@ -533,32 +539,34 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 			}
 
 			/* Add zero padding */
-			if (_NOLIBC_PF_FLAGS_CONTAIN(flags, '0', '.')) {
-				if (!_NOLIBC_PF_FLAGS_CONTAIN(flags, '.')) {
-					if (_NOLIBC_PF_FLAGS_CONTAIN(flags, '-'))
-						/* Left justify overrides zero pad */
-						goto prepend_sign;
-					/* eg "%05d", Zero pad to field width less sign.
-					 * Note that precision can end up negative so all
-					 * the variables have to be 'signed int'.
-					 */
-					precision = width;
-					if (sign_prefix) {
+			if (precision < 0) {
+				/* No explicit precision (or negative from "%.*s"). */
+				if (!_NOLIBC_PF_FLAGS_CONTAIN(flags, '0'))
+					goto no_zero_padding;
+				if (_NOLIBC_PF_FLAGS_CONTAIN(flags, '-'))
+					/* Left justify overrides zero pad */
+					goto no_zero_padding;
+				/* eg "%05d", Zero pad to field width less sign.
+				 * Note that precision can end up negative so all
+				 * the variables have to be 'signed int'.
+				 */
+				precision = width;
+				if (sign_prefix) {
+					precision--;
+					if (sign_prefix >= 256)
 						precision--;
-						if (sign_prefix >= 256)
-							precision--;
-					}
-				}
-				if (precision > 31)
-					/* Don't run off the start of outbuf[], arbitrary limit
-					 * longer than the longest number field. */
-					precision = 31;
-				for (; len < precision; len++) {
-					/* Stop gcc generating horrid code and memset(). */
-					_NOLIBC_OPTIMIZER_HIDE_VAR(len);
-					*--out = '0';
 				}
 			}
+			if (precision > 31)
+				/* Don't run off the start of outbuf[], arbitrary limit
+				 * longer than the longest number field. */
+				precision = 31;
+			for (; len < precision; len++) {
+				/* Stop gcc generating horrid code and memset(). */
+				_NOLIBC_OPTIMIZER_HIDE_VAR(len);
+				*--out = '0';
+			}
+no_zero_padding:
 
 			/* %#o has set sign_prefix to '0', but we don't want so add an extra
 			 * leading zero here.
@@ -603,7 +611,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 
 do_strlen_output:
 		/* Open coded strnlen() (slightly smaller). */
-		for (len = 0; len < precision; len++)
+		for (len = 0; precision < 0 || len < precision; len++)
 			if (!outstr[len])
 				break;
 
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 0ca695acbc44..83082486b8c3 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1825,7 +1825,7 @@ static int run_printf(int min, int max)
 		CASE_TEST(char);         EXPECT_VFPRINTF(1, "|c|d|   e|", "|%c|%.0c|%4c|", 'c', 'd', 'e'); break;
 		CASE_TEST(octal);        EXPECT_VFPRINTF(1, "|17|  0033||", "|%o|%6.4o|%.0o|", 017, 033, 0); break;
 		CASE_TEST(octal_max);    EXPECT_VFPRINTF(1, "1777777777777777777777", "%llo", ~0ULL); break;
-		CASE_TEST(octal_alt);    EXPECT_VFPRINTF(1, "|0|01|02|034|0|", "|%#o|%#o|%#02o|%#02o|%#.0o|", 0, 1, 2, 034, 0); break;
+		CASE_TEST(octal_alt);    EXPECT_VFPRINTF(1, "|0|01|02|034|0|0|", "|%#o|%#o|%#02o|%#02o|%#.0o|%0-#o|", 0, 1, 2, 034, 0, 0); break;
 		CASE_TEST(hex_nolibc);   EXPECT_VFPRINTF(is_nolibc, "|f|d|", "|%x|%X|", 0xf, 0xd); break;
 		CASE_TEST(hex_libc);     EXPECT_VFPRINTF(!is_nolibc, "|f|D|", "|%x|%X|", 0xf, 0xd); break;
 		CASE_TEST(hex_alt);      EXPECT_VFPRINTF(1, "|0x1|  0x2|    0|", "|%#x|%#5x|%#5x|", 1, 2, 0); break;
@@ -1843,6 +1843,7 @@ static int run_printf(int min, int max)
 		CASE_TEST(truncation);   EXPECT_VFPRINTF(1, "012345678901234567890123456789", "%s", "012345678901234567890123456789"); break;
 		CASE_TEST(string_width); EXPECT_VFPRINTF(1, "         1", "%10s", "1"); break;
 		CASE_TEST(string_trunc); EXPECT_VFPRINTF(1, "     12345", "%10.5s", "1234567890"); break;
+		CASE_TEST(string_var);   EXPECT_VFPRINTF(1, "| ab|ef | ij|kl |", "|%*.*s|%*.*s|%*.*s|%*.*s|", 3, 2, "abcd", -3, 2, "efgh", 3, -1, "ij", -3, -1, "kl"); break;
 		CASE_TEST(number_width); EXPECT_VFPRINTF(1, "         1", "%10d", 1); break;
 		CASE_TEST(number_left);  EXPECT_VFPRINTF(1, "|-5      |", "|%-8d|", -5); break;
 		CASE_TEST(string_align); EXPECT_VFPRINTF(1, "|foo     |", "|%-8s|", "foo"); break;
@@ -1856,7 +1857,7 @@ static int run_printf(int min, int max)
 		CASE_TEST(num_p_tr_libc);EXPECT_VFPRINTF(!is_nolibc, "00000000000000000000000000000000005", "%035d", 5); break;
 		CASE_TEST(number_prec);  EXPECT_VFPRINTF(1, "     00005", "%10.5d", 5); break;
 		CASE_TEST(num_prec_neg); EXPECT_VFPRINTF(1, "    -00005", "%10.5d", -5); break;
-		CASE_TEST(num_prec_var); EXPECT_VFPRINTF(1, "    -00005", "%*.*d", 10, 5, -5); break;
+		CASE_TEST(number_var);   EXPECT_VFPRINTF(1, "|    -00005|5 |", "|%*.*d|%*.*d|", 10, 5, -5, -2, -10, 5); break;
 		CASE_TEST(num_0_prec_0); EXPECT_VFPRINTF(1, "|| |+||||", "|%.0d|% .0d|%+.0d|%.0u|%.0x|%#.0x|", 0, 0, 0, 0, 0, 0); break;
 		CASE_TEST(errno);        errno = 22; EXPECT_VFPRINTF(is_nolibc, "errno=22", "%m"); break;
 		CASE_TEST(errno-neg);    errno = -22; EXPECT_VFPRINTF(is_nolibc, "errno=-22   ", "%-12m"); break;
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] tools/nolibc/printf: Support negative variable width and precision
  2026-03-23 11:22 [PATCH 1/1] tools/nolibc/printf: Support negative variable width and precision david.laight.linux
@ 2026-03-25 20:37 ` Thomas Weißschuh
  2026-03-25 23:03   ` David Laight
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Weißschuh @ 2026-03-25 20:37 UTC (permalink / raw)
  To: david.laight.linux; +Cc: Willy Tarreau, linux-kernel

Hi David,

thanks again for your patch!

On 2026-03-23 11:22:47+0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> For (eg) "%*.*s" treat a negative field width as a request to left align
> the output (the same as the '-' flag), and a negative precision to
> request the default precision.

This makes sense, so far so good.

> Set the default precision to -1 (not INT_MAX) and add explicit checks
> to the string handling for negative values (makes the tet unsigned).

'tet'?

> For numeric output check for 'precision >= 0' instead of testing
> _NOLIBC_PF_FLAGS_CONTAIN(flags, '.').
> This needs an inverted test, some extra goto and removes an indentation.
> The changed conditionals fix printf("%0-#o", 0) - but '0' and '-' shouldn't
> both be specified.

Is this also related to the negative field width as described above?
If yes, could you explain how? If not, please split it out.
In general, the smaller the patches, the easier the review.

> Best viewed with 'git diff -b' after being commited.

This should go after '---'. No reason on its own to resend, though.

> Additional test cases added.

No need to mention that, it is obvious from the diff :-)

> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> 
> I missed this bit in the earlier patches.
> Size wise it is pretty neutral. It really seems to depend on how many registers
> get saved across the call to _nolibc_u64toa_base() - gcc doesn't seem to use
> the correct registers to avoid spills.
> 
> I did look at whether making 'width' negative at the top was better than
> keeping a '-' flag - but it bloats things because you need the absolute value
> at the bottom.
> 
>  tools/include/nolibc/stdio.h                 | 68 +++++++++++---------
>  tools/testing/selftests/nolibc/nolibc-test.c |  5 +-
>  2 files changed, 41 insertions(+), 32 deletions(-)
> 
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index 8f7e1948a651..b6d14a58cfe7 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
> @@ -347,6 +347,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
>  	char *out;
>  	const char *outstr;
>  	unsigned int sign_prefix;
> +	int got_width;

bool?

(...)

Otherwise looks good from what I understand.
But smaller patches would really give me more confidence.


Thomas

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] tools/nolibc/printf: Support negative variable width and precision
  2026-03-25 20:37 ` Thomas Weißschuh
@ 2026-03-25 23:03   ` David Laight
  0 siblings, 0 replies; 3+ messages in thread
From: David Laight @ 2026-03-25 23:03 UTC (permalink / raw)
  To: Thomas Weißschuh; +Cc: Willy Tarreau, linux-kernel

On Wed, 25 Mar 2026 21:37:11 +0100
Thomas Weißschuh <linux@weissschuh.net> wrote:

> Hi David,
> 
> thanks again for your patch!
> 
> On 2026-03-23 11:22:47+0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > For (eg) "%*.*s" treat a negative field width as a request to left align
> > the output (the same as the '-' flag), and a negative precision to
> > request the default precision.  
> 
> This makes sense, so far so good.
> 
> > Set the default precision to -1 (not INT_MAX) and add explicit checks
> > to the string handling for negative values (makes the tet unsigned).  
> 
> 'tet'?

test - I wanted to say that the test:
	len = precision < 0 || precision >= 6 ? 6 : 0;
isn't actually a double comparison.
Perhaps it isn't needed.

> 
> > For numeric output check for 'precision >= 0' instead of testing
> > _NOLIBC_PF_FLAGS_CONTAIN(flags, '.').
> > This needs an inverted test, some extra goto and removes an indentation.
> > The changed conditionals fix printf("%0-#o", 0) - but '0' and '-' shouldn't
> > both be specified.  
> 
> Is this also related to the negative field width as described above?
> If yes, could you explain how? If not, please split it out.
> In general, the smaller the patches, the easier the review.

Everything except the 'if (width < 0)' test is one change.
It also changes v5 15/17, I did it as a delta (rather than a replacement
patch) because of the later changes.

The fix for printf("%0-#o", 0) (without these changes it generates "00")
happens because of the way the conditionals and gotos change.
Patch v5 15/17 that added zero padding and field precision had two
'goto prepend_sign', patch 16 (add octal) needs them to go opposite
sides of the 'if (sign_prefix == *out)' test.
With the changed conditionals you need both labels or ("%#o", 0)
always goes wrong.

> 
> > Best viewed with 'git diff -b' after being commited.  
> 
> This should go after '---'. No reason on its own to resend, though.
> 
> > Additional test cases added.  
> 
> No need to mention that, it is obvious from the diff :-)
> 
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> > 
> > I missed this bit in the earlier patches.
> > Size wise it is pretty neutral. It really seems to depend on how many registers
> > get saved across the call to _nolibc_u64toa_base() - gcc doesn't seem to use
> > the correct registers to avoid spills.
> > 
> > I did look at whether making 'width' negative at the top was better than
> > keeping a '-' flag - but it bloats things because you need the absolute value
> > at the bottom.
> > 
> >  tools/include/nolibc/stdio.h                 | 68 +++++++++++---------
> >  tools/testing/selftests/nolibc/nolibc-test.c |  5 +-
> >  2 files changed, 41 insertions(+), 32 deletions(-)
> > 
> > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > index 8f7e1948a651..b6d14a58cfe7 100644
> > --- a/tools/include/nolibc/stdio.h
> > +++ b/tools/include/nolibc/stdio.h
> > @@ -347,6 +347,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
> >  	char *out;
> >  	const char *outstr;
> >  	unsigned int sign_prefix;
> > +	int got_width;  
> 
> bool?

There weren't any others in this file and I'm 'old school'...

	David

> 
> (...)
> 
> Otherwise looks good from what I understand.
> But smaller patches would really give me more confidence.
> 
> 
> Thomas


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-25 23:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-23 11:22 [PATCH 1/1] tools/nolibc/printf: Support negative variable width and precision david.laight.linux
2026-03-25 20:37 ` Thomas Weißschuh
2026-03-25 23:03   ` David Laight

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox