[PATCH next 00/12] tools/nolibc: Enhance printf()

[PATCH next 00/12] tools/nolibc: Enhance printf()

[david.laight.linux]

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

Update printf() so that it handles almost all the non-fp formats.
In particular:
- Left alignment.
- Zero padding.
- Field precision.
- Variable field width and precision.
- Width modifiers q, L, t and z.
- Conversion specifiers i and X (X generates lower case).
About the only thing that is missing is octal.

The tests are updated to match.

There is a slight increase in code size, but it is minimalised
by the the heavy use of bit-pattern matches.

vfprintf() is modified to buffer data for each call and do a single
write system call at the end.
This improves performance somewhat, but is actually most useful when
using strace.

David Laight (12):
  tools/nolibc/printf: Move snprintf length check to callback
  tools/nolibc/printf: Add buffering to vfprintf() callback.
  tools/nolibc: change printf() to output pad in 16 byte chunks
  selftests/nolibc: Improve reporting of vfprintf() errors
  tools/nolibc/stdio: Simplify __nolibc_printf()
  tools/nolibc: Add support for left alignment and %[tzq]d" to printf
  tools/nolibc: printf() prepend the sign after a numeric conversion
  tools/nolibc: use bit-match to detect valid printf conversion
    character
  tools/nolibc: add precision and zero padding to printf()
  tools/nolibc: Use bit-pattern for printf integral formats
  selftests/nolibc: Increase coverage of printf format tests
  selftests/nolibc: Use printf("%.*s", n, "") to align output

 tools/include/nolibc/stdio.h                 | 396 +++++++++++++------
 tools/testing/selftests/nolibc/nolibc-test.c |  96 +++--
 2 files changed, 336 insertions(+), 156 deletions(-)

-- 
2.39.5

[PATCH next 01/12] tools/nolibc/printf: Move length check to snprintf callback

[david.laight.linux]

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

Move output truncation to the snprintf() callback.
This simplifies the main code and ensures the truncation will be
correct when left-alignment is added.

Add a zero length callback to 'finalise' the buffer rather than
doing it in snprintf() itself.

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

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 1f16dab2ac88..6c8fa41dbc01 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -245,15 +245,15 @@ char *fgets(char *s, int size, FILE *stream)
  *  - %s
  *  - unknown modifiers are ignored.
  */
-typedef int (*__nolibc_printf_cb)(intptr_t state, const char *buf, size_t size);
+typedef int (*__nolibc_printf_cb)(void *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)
+static __attribute__((unused, format(printf, 3, 0)))
+int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list args)
 {
 	char escape, lpref, c;
 	unsigned long long v;
 	unsigned int written, width;
-	size_t len, ofs, w;
+	size_t len, ofs;
 	char tmpbuf[21];
 	const char *outstr;
 
@@ -355,17 +355,13 @@ int __nolibc_printf(__nolibc_printf_cb cb, intptr_t state, size_t n, const char
 			outstr = fmt;
 			len = ofs - 1;
 		flush_str:
-			if (n) {
-				w = len < n ? len : n;
-				n -= w;
-				while (width-- > w) {
-					if (cb(state, " ", 1) != 0)
-						return -1;
-					written += 1;
-				}
-				if (cb(state, outstr, w) != 0)
+			while (width-- > len) {
+				if (cb(state, " ", 1) != 0)
 					return -1;
+				written += 1;
 			}
+			if (cb(state, outstr, len) != 0)
+				return -1;
 
 			written += len;
 		do_escape:
@@ -378,18 +374,22 @@ int __nolibc_printf(__nolibc_printf_cb cb, intptr_t state, size_t n, const char
 
 		/* literal char, just queue it */
 	}
+
+	if (cb(state, NULL, 0) != 0)
+		return -1;
+
 	return written;
 }
 
-static int __nolibc_fprintf_cb(intptr_t state, const char *buf, size_t size)
+static int __nolibc_fprintf_cb(void *stream, const char *buf, size_t size)
 {
-	return _fwrite(buf, size, (FILE *)state);
+	return size ? _fwrite(buf, size, stream) : 0;
 }
 
 static __attribute__((unused, format(printf, 2, 0)))
 int vfprintf(FILE *stream, const char *fmt, va_list args)
 {
-	return __nolibc_printf(__nolibc_fprintf_cb, (intptr_t)stream, SIZE_MAX, fmt, args);
+	return __nolibc_printf(__nolibc_fprintf_cb, stream, fmt, args);
 }
 
 static __attribute__((unused, format(printf, 1, 0)))
@@ -447,26 +447,38 @@ int dprintf(int fd, const char *fmt, ...)
 	return ret;
 }
 
-static int __nolibc_sprintf_cb(intptr_t _state, const char *buf, size_t size)
+struct __nolibc_sprintf_cb_state {
+	char *buf;
+	size_t size;
+};
+
+static int __nolibc_sprintf_cb(void *v_state, const char *buf, size_t size)
 {
-	char **state = (char **)_state;
+	struct __nolibc_sprintf_cb_state *state = v_state;
+	char *tgt;
 
-	memcpy(*state, buf, size);
-	*state += size;
+	if (size >= state->size) {
+		if (state->size <= 1)
+			return 0;
+		size = state->size - 1;
+	}
+	tgt = state->buf;
+	if (size) {
+		state->size -= size;
+		state->buf = tgt + size;
+		memcpy(tgt, buf, size);
+	} else {
+		*tgt = '\0';
+	}
 	return 0;
 }
 
 static __attribute__((unused, format(printf, 3, 0)))
 int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 {
-	char *state = buf;
-	int ret;
+	struct __nolibc_sprintf_cb_state state = { .buf = buf, .size = size };
 
-	ret = __nolibc_printf(__nolibc_sprintf_cb, (intptr_t)&state, size, fmt, args);
-	if (ret < 0)
-		return ret;
-	buf[(size_t)ret < size ? (size_t)ret : size - 1] = '\0';
-	return ret;
+	return __nolibc_printf(__nolibc_sprintf_cb, &state, fmt, args);
 }
 
 static __attribute__((unused, format(printf, 3, 4)))
-- 
2.39.5

[PATCH next 02/12] tools/nolibc/printf: Add buffering to vfprintf() callback.

[david.laight.linux]

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

Add per-call buffering to the vprintf() callback.
While this adds some extra code it will speed things up and
makes a massive difference to anyone looking at strace output.

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

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 6c8fa41dbc01..985e10d747db 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -381,15 +381,41 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 	return written;
 }
 
-static int __nolibc_fprintf_cb(void *stream, const char *buf, size_t size)
+struct __nolibc_fprintf_cb_state {
+	FILE *stream;
+	unsigned int buf_offset;
+	char buf[128];
+};
+
+static int __nolibc_fprintf_cb(void *v_state, const char *buf, size_t size)
 {
-	return size ? _fwrite(buf, size, stream) : 0;
+	struct __nolibc_fprintf_cb_state *state = v_state;
+	unsigned int off = state->buf_offset;
+
+	if (off + size > sizeof(state->buf) || buf == NULL) {
+		state->buf_offset = 0;
+		if (off && _fwrite(state->buf, off, state->stream))
+			return -1;
+		if (size > sizeof(state->buf))
+			return _fwrite(buf, size, state->stream);
+		off = 0;
+	}
+
+	if (size) {
+		state->buf_offset = off + size;
+		memcpy(state->buf + off, buf, size);
+	}
+	return 0;
 }
 
 static __attribute__((unused, format(printf, 2, 0)))
 int vfprintf(FILE *stream, const char *fmt, va_list args)
 {
-	return __nolibc_printf(__nolibc_fprintf_cb, stream, fmt, args);
+	struct __nolibc_fprintf_cb_state state;
+
+	state.stream = stream;
+	state.buf_offset = 0;
+	return __nolibc_printf(__nolibc_fprintf_cb, &state, fmt, args);
 }
 
 static __attribute__((unused, format(printf, 1, 0)))
-- 
2.39.5

[PATCH next 03/12] tools/nolibc/printf: output pad spaces in 16 byte chunks

[david.laight.linux]

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

Simple to do and saves calls to the callback function.

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

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 985e10d747db..52cb516ea3db 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -355,10 +355,12 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 			outstr = fmt;
 			len = ofs - 1;
 		flush_str:
-			while (width-- > len) {
-				if (cb(state, " ", 1) != 0)
+			while (width > len) {
+				unsigned int pad_len = ((width - len - 1) & 15) + 1;
+				width -= pad_len;
+				written += pad_len;
+				if (cb(state, "                ", pad_len) != 0)
 					return -1;
-				written += 1;
 			}
 			if (cb(state, outstr, len) != 0)
 				return -1;
-- 
2.39.5

[PATCH next 04/12] selftests/nolibc: Improve reporting of vfprintf() errors

[david.laight.linux]

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

Check the string matches before checking the returned length.
Only print the string once when it matches.
Normally the length is that of the expected string, make it easier
to write tests by treating a length of zero as being that of the
expected output.
Additionally check that nothing beyond the end is written.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 27 ++++++++++++++++----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 3c5a226dad3a..9378a1f26c34 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1567,28 +1567,45 @@ int run_stdlib(int min, int max)
 
 static int expect_vfprintf(int llen, int c, const char *expected, const char *fmt, ...)
 {
+	unsigned int i;
 	char buf[100];
 	va_list args;
 	ssize_t w;
 	int ret;
 
+	for (i = 0; i < sizeof(buf); i++)
+		buf[i] = i;
 
 	va_start(args, fmt);
-	/* Only allow writing 21 bytes, to test truncation */
+	/* Only allow writing 20 bytes, to test truncation */
 	w = vsnprintf(buf, 21, fmt, args);
 	va_end(args);
 
+	llen += printf(" \"%s\"", buf);
+	ret = strcmp(expected, buf);
+	if (ret) {
+		llen += printf(" should be \"%s\"", expected);
+		result(llen, FAIL);
+		return 1;
+	}
+	if (!c)
+		c = strlen(expected);
 	if (w != c) {
 		llen += printf(" written(%d) != %d", (int)w, c);
 		result(llen, FAIL);
 		return 1;
 	}
 
-	llen += printf(" \"%s\" = \"%s\"", expected, buf);
-	ret = strncmp(expected, buf, c);
+	for (i = c + 1; i < sizeof(buf); i++) {
+		if (buf[i] - i) {
+			llen += printf(" overwrote buf[%d] with 0x%x", i, buf[i]);
+			result(llen, FAIL);
+			return 1;
+		}
+	}
 
-	result(llen, ret ? FAIL : OK);
-	return ret;
+	result(llen, OK);
+	return 0;
 }
 
 static int test_scanf(void)
-- 
2.39.5

[PATCH next 05/12] tools/nolibc/printf: Simplify __nolibc_printf()

[david.laight.linux]

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

Move the check for the length modifiers into the format processing
between the field width and conversion specifier.
This lets the loop be simplified and a 'fast scan' for a format start
used.

If an error is detected (eg an invalid conversion specifier) then
copy the invalid format to the output buffer.

Reduces code size by about 10% on x86-64.

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

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 52cb516ea3db..164d2384978e 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -250,28 +250,52 @@ typedef int (*__nolibc_printf_cb)(void *state, const char *buf, size_t size);
 static __attribute__((unused, format(printf, 3, 0)))
 int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list args)
 {
-	char escape, lpref, c;
+	char lpref, c;
 	unsigned long long v;
 	unsigned int written, width;
-	size_t len, ofs;
+	size_t len;
 	char tmpbuf[21];
 	const char *outstr;
 
-	written = ofs = escape = lpref = 0;
+	written = 0;
 	while (1) {
-		c = fmt[ofs++];
+		outstr = fmt;
+		c = *fmt++;
+		if (!c)
+			break;
+
 		width = 0;
+		if (c != '%') {
+			while (*fmt && *fmt != '%')
+				fmt++;
+			len = fmt - outstr;
+		} else {
+			/* we're in a format sequence */
 
-		if (escape) {
-			/* we're in an escape sequence, ofs == 1 */
-			escape = 0;
+			c = *fmt++;
 
 			/* width */
 			while (c >= '0' && c <= '9') {
 				width *= 10;
 				width += c - '0';
 
-				c = fmt[ofs++];
+				c = *fmt++;
+			}
+
+			/* Length modifiers */
+			if (c == 'l') {
+				lpref = 1;
+				c = *fmt++;
+				if (c == 'l') {
+					lpref = 2;
+					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') {
@@ -327,54 +351,30 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 #else
 				outstr = strerror(errno);
 #endif /* NOLIBC_IGNORE_ERRNO */
-			}
-			else if (c == '%') {
-				/* queue it verbatim */
-				continue;
-			}
-			else {
-				/* modifiers or final 0 */
-				if (c == 'l') {
-					/* long format prefix, maintain the escape */
-					lpref++;
-				} else if (c == 'j') {
-					lpref = 2;
-				}
-				escape = 1;
-				goto do_escape;
+			} else {
+				if (c != '%')
+					/* Invalid format, output the format string */
+					fmt = outstr + 1;
+				/* %% is documented as a 'conversion specifier'.
+				 * Any flags, precision or length modifier are ignored.
+				 */
+				width = 0;
+				outstr = "%";
 			}
 			len = strlen(outstr);
-			goto flush_str;
 		}
 
-		/* not an escape sequence */
-		if (c == 0 || c == '%') {
-			/* flush pending data on escape or end */
-			escape = 1;
-			lpref = 0;
-			outstr = fmt;
-			len = ofs - 1;
-		flush_str:
-			while (width > len) {
-				unsigned int pad_len = ((width - len - 1) & 15) + 1;
-				width -= pad_len;
-				written += pad_len;
-				if (cb(state, "                ", pad_len) != 0)
-					return -1;
-			}
-			if (cb(state, outstr, len) != 0)
-				return -1;
+		written += len;
 
-			written += len;
-		do_escape:
-			if (c == 0)
-				break;
-			fmt += ofs;
-			ofs = 0;
-			continue;
+		while (width > len) {
+			unsigned int pad_len = ((width - len - 1) & 15) + 1;
+			width -= pad_len;
+			written += pad_len;
+			if (cb(state, "                ", pad_len) != 0)
+				return -1;
 		}
-
-		/* literal char, just queue it */
+		if (cb(state, outstr, len) != 0)
+			return -1;
 	}
 
 	if (cb(state, NULL, 0) != 0)
-- 
2.39.5

[PATCH next 06/12] tools/nolibc/printf: Add support for left alignment and %[tzLq]d"

[david.laight.linux]

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

[PATCH next 07/12] tools/nolibc/printf: Prepend the sign after a numeric conversion

[david.laight.linux]

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

Needed so that zero can be correctly padded.
Add support for the "+ " modifiers for non-negative integers.

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

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 1ce4d357a802..e4792625c1ec 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -241,7 +241,7 @@ char *fgets(char *s, int size, FILE *stream)
 
 
 /* simple printf(). It supports the following formats:
- *  - %[-][width][{l,t,z,ll,L,j,q}]{d,u,c,x,p,s,m}
+ *  - %[-+ ][width][{l,t,z,ll,L,j,q}]{d,u,c,x,p,s,m}
  *  - %%
  *  - invalid formats are copied to the output buffer
  */
@@ -254,7 +254,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 	char c;
 	int len, written, width;
 	unsigned int flags;
-	char tmpbuf[21];
+	char tmpbuf[64];
 	const char *outstr;
 
 	written = 0;
@@ -277,7 +277,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 
 			/* Flag characters */
 			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
-				if ((__PF_FLAG(c) & (__PF_FLAG('-'))) == 0)
+				if ((__PF_FLAG(c) & (__PF_FLAG('-') | __PF_FLAG(' ') | __PF_FLAG('+'))) == 0)
 					break;
 				flags |= __PF_FLAG(c);
 			}
@@ -307,7 +307,8 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 			if (c == 'c' || c == 'd' || c == 'u' || c == 'x' || c == 'p') {
 				unsigned long long v;
 				long long signed_v;
-				char *out = tmpbuf;
+				char *out = tmpbuf + 32;
+				int sign = 0;
 
 				if ((c == 'p') || (flags & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z')))) {
 					v = va_arg(args, unsigned long);
@@ -322,24 +323,35 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 
 				switch (c) {
 				case 'c':
-					out[0] = v;
-					out[1] = 0;
-					break;
+					tmpbuf[0] = v;
+					len = 1;
+					outstr = tmpbuf;
+					goto do_output;
 				case 'd':
-					i64toa_r(signed_v, out);
-					break;
+					if (signed_v < 0) {
+						sign = '-';
+						v = -(signed_v + 1);
+						v++;
+					} else if (flags & __PF_FLAG('+')) {
+						sign = '+';
+					} else if (flags & __PF_FLAG(' ')) {
+						sign = ' ';
+					}
+					__nolibc_fallthrough;
 				case 'u':
 					u64toa_r(v, out);
 					break;
 				case 'p':
-					*(out++) = '0';
-					*(out++) = 'x';
+					sign = 'x' | '0' << 8;
 					__nolibc_fallthrough;
 				default: /* 'x' and 'p' above */
 					u64toh_r(v, out);
 					break;
 				}
-				outstr = tmpbuf;
+				for (; sign; sign >>= 8) {
+					*--out = sign;
+				}
+				outstr = out;
 			}
 			else if (c == 's') {
 				outstr = va_arg(args, char *);
@@ -365,6 +377,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 			len = strlen(outstr);
 		}
 
+do_output:
 		written += len;
 
                 /* An OPTIMIZER_HIDE_VAR() seems to stop gcc back-merging this
-- 
2.39.5

[PATCH next 08/12] tools/nolibc/printf: use bit-match to detect valid conversion characters

[david.laight.linux]

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

Move %s inside the numeric block to optimise away one va_arg() call.
Add support for %i, an alias for %d.
Add support for %X, as an alias for %x (works but lower case A..F).

Use the length returned by u64toa_r() to avoid strlen() for everything
except %s and %m.

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

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index e4792625c1ec..d8b6184ff0f3 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -241,7 +241,7 @@ char *fgets(char *s, int size, FILE *stream)
 
 
 /* simple printf(). It supports the following formats:
- *  - %[-+ ][width][{l,t,z,ll,L,j,q}]{d,u,c,x,p,s,m}
+ *  - %[-+ ][width][{l,t,z,ll,L,j,q}]{d,i,u,c,x,X,p,s,m}
  *  - %%
  *  - invalid formats are copied to the output buffer
  */
@@ -254,7 +254,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 	char c;
 	int len, written, width;
 	unsigned int flags;
-	char tmpbuf[64];
+	char tmpbuf[32 + 24];
 	const char *outstr;
 
 	written = 0;
@@ -304,13 +304,21 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 				}
 			}
 
-			if (c == 'c' || c == 'd' || c == 'u' || c == 'x' || c == 'p') {
+			/* Conversion specifiers are lower case except 'X' treated as 'x' */
+			if (!((c >= 'a' && c <= 'z') || (c == 'X' && (c = 'x'))))
+				goto bad_conversion_specifier;
+
+			/* Conversion specifiers */
+			if (__PF_FLAG(c) & (__PF_FLAG('c') | __PF_FLAG('d') | __PF_FLAG('i') | __PF_FLAG('u') |
+					    __PF_FLAG('x') | __PF_FLAG('p') | __PF_FLAG('s'))) {
 				unsigned long long v;
 				long long signed_v;
 				char *out = tmpbuf + 32;
 				int sign = 0;
 
-				if ((c == 'p') || (flags & (__PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z')))) {
+				/* Annoying 'p' === '0' so mask from flags */
+				if ((__PF_FLAG(c) | (flags & ~__PF_FLAG('p'))) &
+				    (__PF_FLAG('p') | __PF_FLAG('s') | __PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z'))) {
 					v = va_arg(args, unsigned long);
 					signed_v = (long)v;
 				} else if (flags & (__PF_FLAG('j') | __PF_FLAG('q'))) {
@@ -327,7 +335,18 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 					len = 1;
 					outstr = tmpbuf;
 					goto do_output;
+				case 's':
+					if (!v) {
+						outstr = "(null)";
+						len = 6;
+						goto do_output;
+					}
+					outstr = (void *)v;
+do_strnlen_output:
+					len = strnlen(outstr, INT_MAX);
+					goto do_output;
 				case 'd':
+				case 'i':
 					if (signed_v < 0) {
 						sign = '-';
 						v = -(signed_v + 1);
@@ -339,42 +358,41 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 					}
 					__nolibc_fallthrough;
 				case 'u':
-					u64toa_r(v, out);
+					len = u64toa_r(v, out);
 					break;
 				case 'p':
 					sign = 'x' | '0' << 8;
 					__nolibc_fallthrough;
 				default: /* 'x' and 'p' above */
-					u64toh_r(v, out);
+					len = u64toh_r(v, out);
 					break;
 				}
 				for (; sign; sign >>= 8) {
+					len++;
 					*--out = sign;
 				}
 				outstr = out;
 			}
-			else if (c == 's') {
-				outstr = va_arg(args, char *);
-				if (!outstr)
-					outstr="(null)";
-			}
 			else if (c == 'm') {
 #ifdef NOLIBC_IGNORE_ERRNO
 				outstr = "unknown error";
+				len = __builtin_strlen(outstr);
 #else
 				outstr = strerror(errno);
+				goto do_strnlen_output;
 #endif /* NOLIBC_IGNORE_ERRNO */
 			} else {
+bad_conversion_specifier:
 				if (c != '%')
 					/* Invalid format, output the format string */
 					fmt = outstr + 1;
 				/* %% is documented as a 'conversion specifier'.
 				 * Any flags, precision or length modifier are ignored.
 				 */
+				outstr = fmt - 1;
+				len = 1;
 				width = 0;
-				outstr = "%";
 			}
-			len = strlen(outstr);
 		}
 
 do_output:
-- 
2.39.5

[PATCH next 09/12] tools/nolibc/printf: support precision and zero padding

[david.laight.linux]

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

Support '*' for both length and precision.
Output "(nil)" for NULL pointers (matches glibc).
Output "0" not "0x0" for printf("%#x", 0).
Output no digits for printf("%.0d", 0).

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

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index d8b6184ff0f3..bad528921590 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -240,10 +240,15 @@ char *fgets(char *s, int size, FILE *stream)
 }
 
 
-/* simple printf(). It supports the following formats:
- *  - %[-+ ][width][{l,t,z,ll,L,j,q}]{d,i,u,c,x,X,p,s,m}
- *  - %%
- *  - invalid formats are copied to the output buffer
+/* printf(). Supports most of the normal integer and string formats.
+ *  - %[#0-+ ][width|*[.precision|*]][{l,t,z,ll,L,j,q}]{d,i,u,c,x,X,p,s,m}
+ *  - %% generates a single %
+ *  - %m outputs strerror(errno).
+ *  - # only affects %x and prepends 0x to non-zero values.
+ *  - %o (octal) isn't supported.
+ *  - %X outputs a..f the same as %x.
+ *  - No support for wide characters.
+ *  - invalid formats are copied to the output buffer.
  */
 typedef int (*__nolibc_printf_cb)(void *state, const char *buf, size_t size);
 
@@ -252,7 +257,7 @@ static __attribute__((unused, format(printf, 3, 0)))
 int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list args)
 {
 	char c;
-	int len, written, width;
+	int len, written, width, precision;
 	unsigned int flags;
 	char tmpbuf[32 + 24];
 	const char *outstr;
@@ -277,17 +282,30 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 
 			/* Flag characters */
 			for (; c >= 0x20 && c <= 0x3f; c = *fmt++) {
-				if ((__PF_FLAG(c) & (__PF_FLAG('-') | __PF_FLAG(' ') | __PF_FLAG('+'))) == 0)
+				if ((__PF_FLAG(c) & (__PF_FLAG('-') | __PF_FLAG(' ') | __PF_FLAG('+') |
+						     __PF_FLAG('#') | __PF_FLAG('0'))) == 0)
 					break;
 				flags |= __PF_FLAG(c);
 			}
 
-			/* width */
-			while (c >= '0' && c <= '9') {
-				width *= 10;
-				width += c - '0';
-
-				c = *fmt++;
+			/* width and precision */
+			for (;; c = *fmt++) {
+				if (c == '*') {
+					precision = va_arg(args, unsigned int);
+					c = *fmt++;
+				} else {
+					for (precision = 0; c >= '0' && c <= '9'; c = *fmt++)
+						precision = precision * 10 + (c - '0');
+				}
+				if (flags & __PF_FLAG('.'))
+					break;
+				width = precision;
+				if (c != '.') {
+					/* Default precision for strings */
+					precision = INT_MAX;
+					break;
+				}
+				flags |= __PF_FLAG('.');
 			}
 
 			/* Length modifiers are lower case except 'L' which is the same a 'q' */
@@ -308,7 +326,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 			if (!((c >= 'a' && c <= 'z') || (c == 'X' && (c = 'x'))))
 				goto bad_conversion_specifier;
 
-			/* Conversion specifiers */
+			/* Numeric and pointer conversion specifiers */
 			if (__PF_FLAG(c) & (__PF_FLAG('c') | __PF_FLAG('d') | __PF_FLAG('i') | __PF_FLAG('u') |
 					    __PF_FLAG('x') | __PF_FLAG('p') | __PF_FLAG('s'))) {
 				unsigned long long v;
@@ -338,12 +356,13 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 				case 's':
 					if (!v) {
 						outstr = "(null)";
-						len = 6;
+						/* Match glibc, nothing output if precision too small */
+						len = precision >= 6 ? 6 : 0;
 						goto do_output;
 					}
 					outstr = (void *)v;
 do_strnlen_output:
-					len = strnlen(outstr, INT_MAX);
+					len = strnlen(outstr, precision);
 					goto do_output;
 				case 'd':
 				case 'i':
@@ -356,17 +375,62 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 					} else if (flags & __PF_FLAG(' ')) {
 						sign = ' ';
 					}
-					__nolibc_fallthrough;
-				case 'u':
-					len = u64toa_r(v, out);
-					break;
-				case 'p':
-					sign = 'x' | '0' << 8;
-					__nolibc_fallthrough;
-				default: /* 'x' and 'p' above */
-					len = u64toh_r(v, out);
-					break;
+					c = 'u';
+				}
+
+				if (v == 0) {
+					/* There are special rules for zero. */
+					if (c == 'p') {
+						/* match glibc, precision is ignored */
+						outstr = "(nil)";
+						len = 5;
+						goto do_output;
+					}
+					if (!precision) {
+						/* Explicit %nn.0d, no digits output */
+						len = 0;
+						goto prepend_sign;
+					}
+					/* "#x" should output "0" not "0x0" */
+					*out = '0';
+					len = 1;
+				} else {
+					if (c == 'u') {
+						len = u64toa_r(v, out);
+					} else {
+						len = u64toh_r(v, out);
+						if (c == 'p' || (flags & __PF_FLAG('#')))
+							sign = 'x' | '0' << 8;
+					}
 				}
+
+				/* Add zero padding */
+				if (flags & (__PF_FLAG('0') | __PF_FLAG('.'))) {
+					if (!(flags & __PF_FLAG('.'))) {
+						if (flags & __PF_FLAG('-'))
+							/* Left justify overrides zero pad */
+							goto prepend_sign;
+						/* Zero pad to field width less sign */
+						precision = width;
+						if (sign) {
+							precision--;
+							if (sign >= 256)
+								precision--;
+						}
+					}
+					if (precision > 30)
+						/* Don't run off the start of tmpbuf[] */
+						precision = 30;
+					for (; len < precision; len++) {
+						/* Stop gcc generating horrid code and memset().
+						 * This is OPTIMIZER_HIDE_VAR() from compiler.h.
+						 */
+						__asm__ volatile("" : "=r"(len) : "0"(len));
+						*--out = '0';
+					}
+				}
+
+prepend_sign:
 				for (; sign; sign >>= 8) {
 					len++;
 					*--out = sign;
-- 
2.39.5

[PATCH next 10/12] tools/nolibc/printf: Use bit-pattern for integral formats

[david.laight.linux]

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

Generates better code when code is common for multiple formats.
In particular it stops two calls to u64toa_r().

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

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index bad528921590..23fe8b8e7767 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -241,7 +241,7 @@ char *fgets(char *s, int size, FILE *stream)
 
 
 /* printf(). Supports most of the normal integer and string formats.
- *  - %[#0-+ ][width|*[.precision|*]][{l,t,z,ll,L,j,q}]{d,i,u,c,x,X,p,s,m}
+ *  - %[#0-+ ][width|*[.precision|*]][{l,t,z,ll,L,j,q}]{d,i,u,c,x,X,p,s,m,%}
  *  - %% generates a single %
  *  - %m outputs strerror(errno).
  *  - # only affects %x and prepends 0x to non-zero values.
@@ -258,7 +258,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 {
 	char c;
 	int len, written, width, precision;
-	unsigned int flags;
+	unsigned int flags, c_flag;
 	char tmpbuf[32 + 24];
 	const char *outstr;
 
@@ -326,8 +326,12 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 			if (!((c >= 'a' && c <= 'z') || (c == 'X' && (c = 'x'))))
 				goto bad_conversion_specifier;
 
-			/* Numeric and pointer conversion specifiers */
-			if (__PF_FLAG(c) & (__PF_FLAG('c') | __PF_FLAG('d') | __PF_FLAG('i') | __PF_FLAG('u') |
+			/* Numeric and pointer conversion specifiers.
+			 * We need to check for "%p" or "%#x" later, merging here gives better code.
+			 * But '#' collides with 'c' so shift right.
+			 */
+			c_flag = __PF_FLAG(c) | (flags & __PF_FLAG('#')) >> 1;
+			if (c_flag & (__PF_FLAG('c') | __PF_FLAG('d') | __PF_FLAG('i') | __PF_FLAG('u') |
 					    __PF_FLAG('x') | __PF_FLAG('p') | __PF_FLAG('s'))) {
 				unsigned long long v;
 				long long signed_v;
@@ -335,7 +339,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 				int sign = 0;
 
 				/* Annoying 'p' === '0' so mask from flags */
-				if ((__PF_FLAG(c) | (flags & ~__PF_FLAG('p'))) &
+				if ((c_flag | (flags & ~__PF_FLAG('p'))) &
 				    (__PF_FLAG('p') | __PF_FLAG('s') | __PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z'))) {
 					v = va_arg(args, unsigned long);
 					signed_v = (long)v;
@@ -347,13 +351,14 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 					signed_v = (int)v;
 				}
 
-				switch (c) {
-				case 'c':
+				if (c_flag & __PF_FLAG('c')) {
 					tmpbuf[0] = v;
 					len = 1;
 					outstr = tmpbuf;
 					goto do_output;
-				case 's':
+				}
+
+				if (c_flag & __PF_FLAG('s')) {
 					if (!v) {
 						outstr = "(null)";
 						/* Match glibc, nothing output if precision too small */
@@ -364,8 +369,9 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 do_strnlen_output:
 					len = strnlen(outstr, precision);
 					goto do_output;
-				case 'd':
-				case 'i':
+				}
+
+				if (c_flag & (__PF_FLAG('d') | __PF_FLAG('i'))) {
 					if (signed_v < 0) {
 						sign = '-';
 						v = -(signed_v + 1);
@@ -375,12 +381,11 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 					} else if (flags & __PF_FLAG(' ')) {
 						sign = ' ';
 					}
-					c = 'u';
 				}
 
 				if (v == 0) {
 					/* There are special rules for zero. */
-					if (c == 'p') {
+					if (c_flag & __PF_FLAG('p')) {
 						/* match glibc, precision is ignored */
 						outstr = "(nil)";
 						len = 5;
@@ -395,11 +400,11 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list
 					*out = '0';
 					len = 1;
 				} else {
-					if (c == 'u') {
+					if (c_flag & (__PF_FLAG('d') | __PF_FLAG('i') | __PF_FLAG('u'))) {
 						len = u64toa_r(v, out);
 					} else {
 						len = u64toh_r(v, out);
-						if (c == 'p' || (flags & __PF_FLAG('#')))
+						if (c_flag & (__PF_FLAG('p') | __PF_FLAG('#' - 1)))
 							sign = 'x' | '0' << 8;
 					}
 				}
-- 
2.39.5

[PATCH 11/12] selftests/nolibc: Increase coverage of printf format tests

[david.laight.linux]

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

Extra tests include:
- %%, including ignored modifiers.
- Invalid formats copied to output buffer (matches glibc).
- Left aligned output "%-..."
- Zero padding "%0...".
- "(nil)" for NULL pointers (matches glibc).
- Alternate form "%#x" (prepends 0x in non-zero).
- Field precision as well as width, printf("%.0d", 0) is "".
- Variable length width and precision "%*.*d".
- Length qualifiers L and ll.
- Conversion specifiers i and X.
- More 'corner' cases.

There are no explicit tests of long (l, t or z) because they would
have to differ between 32bit and 64bit.

Set the expected length to zero for all the non-truncating tests.
(Annoying when a test is changed.)

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 48 +++++++++++++++-----
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 9378a1f26c34..bf92b9046175 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1565,6 +1565,7 @@ int run_stdlib(int min, int max)
 #define EXPECT_VFPRINTF(c, expected, fmt, ...)				\
 	ret += expect_vfprintf(llen, c, expected, fmt, ##__VA_ARGS__)
 
+__attribute__((mat(printf, 3, 0)))
 static int expect_vfprintf(int llen, int c, const char *expected, const char *fmt, ...)
 {
 	unsigned int i;
@@ -1727,20 +1728,43 @@ static int run_printf(int min, int max)
 		 */
 		switch (test + __LINE__ + 1) {
 		CASE_TEST(empty);        EXPECT_VFPRINTF(0, "", ""); break;
-		CASE_TEST(simple);       EXPECT_VFPRINTF(3, "foo", "foo"); break;
-		CASE_TEST(string);       EXPECT_VFPRINTF(3, "foo", "%s", "foo"); break;
-		CASE_TEST(number);       EXPECT_VFPRINTF(4, "1234", "%d", 1234); break;
-		CASE_TEST(negnumber);    EXPECT_VFPRINTF(5, "-1234", "%d", -1234); break;
-		CASE_TEST(unsigned);     EXPECT_VFPRINTF(5, "12345", "%u", 12345); break;
-		CASE_TEST(char);         EXPECT_VFPRINTF(1, "c", "%c", 'c'); break;
-		CASE_TEST(hex);          EXPECT_VFPRINTF(1, "f", "%x", 0xf); break;
-		CASE_TEST(pointer);      EXPECT_VFPRINTF(3, "0x1", "%p", (void *) 0x1); break;
-		CASE_TEST(uintmax_t);    EXPECT_VFPRINTF(20, "18446744073709551615", "%ju", 0xffffffffffffffffULL); break;
-		CASE_TEST(intmax_t);     EXPECT_VFPRINTF(20, "-9223372036854775807", "%jd", 0x8000000000000001LL); break;
+		CASE_TEST(simple);       EXPECT_VFPRINTF(0, "foo", "foo"); break;
+		CASE_TEST(string);       EXPECT_VFPRINTF(0, "foo", "%s", "foo"); break;
+		CASE_TEST(number);       EXPECT_VFPRINTF(0, "1234", "%d", 1234); break;
+		CASE_TEST(negnumber);    EXPECT_VFPRINTF(0, "-1234", "%d", -1234); break;
+		CASE_TEST(unsigned);     EXPECT_VFPRINTF(0, "12345", "%u", 12345); break;
+		CASE_TEST(signed_max);   EXPECT_VFPRINTF(0, "2147483647", "%i", ~0u >> 1); break;
+		CASE_TEST(signed_min);   EXPECT_VFPRINTF(0, "-2147483648", "%i", (~0u >> 1) + 1); break;
+		CASE_TEST(unsigned_max); EXPECT_VFPRINTF(0, "4294967295", "%u", ~0u); break;
+		CASE_TEST(char);         EXPECT_VFPRINTF(0, "|c|d|   e|", "|%c|%.0c|%4c|", 'c', 'd', 'e'); break;
+		CASE_TEST(hex);          EXPECT_VFPRINTF(0, "|f|d|", "|%x|%X|", 0xf, 0xd); break;
+		CASE_TEST(pointer);      EXPECT_VFPRINTF(0, "0x1", "%p", (void *) 0x1); break;
+		CASE_TEST(pointer_NULL); EXPECT_VFPRINTF(0, "|(nil)|(nil)|", "|%p|%.4p|", (void *)0, (void *)0); break;
+		CASE_TEST(string_NULL);  EXPECT_VFPRINTF(0, "|(null)||(null)|", "|%s|%.5s|%.6s|", (void *)0, (void *)0, (void *)0); break;
+		CASE_TEST(percent);      EXPECT_VFPRINTF(0, "a%d42%69%", "a%%d%d%%%d%%", 42, 69); break;
+		CASE_TEST(perc_qual);    EXPECT_VFPRINTF(0, "a%d2", "a%-14l%d%d", 2); break;
+		CASE_TEST(invalid);      EXPECT_VFPRINTF(0, "a%12yx3%y42%y", "a%12yx%d%y%d%y", 3, 42); break;
+		CASE_TEST(intmax_max);   EXPECT_VFPRINTF(0, "9223372036854775807", "%lld", ~0ULL >> 1); break;
+		CASE_TEST(intmax_min);   EXPECT_VFPRINTF(0, "-9223372036854775808", "%Li", (~0ULL >> 1) + 1); break;
+		CASE_TEST(uintmax_max);  EXPECT_VFPRINTF(0, "18446744073709551615", "%ju", ~0ULL); break;
 		CASE_TEST(truncation);   EXPECT_VFPRINTF(25, "01234567890123456789", "%s", "0123456789012345678901234"); break;
-		CASE_TEST(string_width); EXPECT_VFPRINTF(10, "         1", "%10s", "1"); break;
-		CASE_TEST(number_width); EXPECT_VFPRINTF(10, "         1", "%10d", 1); break;
+		CASE_TEST(string_width); EXPECT_VFPRINTF(0, "         1", "%10s", "1"); break;
+		CASE_TEST(string_trunc); EXPECT_VFPRINTF(0, "     12345", "%10.5s", "1234567890"); break;
+		CASE_TEST(number_width); EXPECT_VFPRINTF(0, "         1", "%10d", 1); break;
+		CASE_TEST(number_left);  EXPECT_VFPRINTF(0, "|-5      |", "|%-8d|", -5); break;
+		CASE_TEST(string_align); EXPECT_VFPRINTF(0, "|foo     |", "|%-8s|", "foo"); break;
 		CASE_TEST(width_trunc);  EXPECT_VFPRINTF(25, "                    ", "%25d", 1); break;
+		CASE_TEST(width_tr_lft); EXPECT_VFPRINTF(25, "1                   ", "%-25d", 1); break;
+		CASE_TEST(number_pad);   EXPECT_VFPRINTF(0, "0000000005", "%010d", 5); break;
+		CASE_TEST(num_pad_neg);  EXPECT_VFPRINTF(0, "-000000005", "%010d", -5); break;
+		CASE_TEST(num_pad_hex);  EXPECT_VFPRINTF(0, "00fffffffb", "%010x", -5); break;
+		CASE_TEST(number_prec);  EXPECT_VFPRINTF(0, "     00005", "%10.5d", 5); break;
+		CASE_TEST(num_prec_neg); EXPECT_VFPRINTF(0, "    -00005", "%10.5d", -5); break;
+		CASE_TEST(num_prec_var); EXPECT_VFPRINTF(0, "    -00005", "%*.*d", 10, 5, -5); break;
+		CASE_TEST(num_0_prec_0); EXPECT_VFPRINTF(0, "|| |+||||", "|%.0d|% .0d|%+.0d|%.0u|%.0x|%#.0x|", 0, 0, 0, 0, 0, 0); break;
+		CASE_TEST(hex_alt);      EXPECT_VFPRINTF(0, "|0x1|0x01| 0x02|", "|%#x|%#04x|%#5.2x|", 1, 1, 2); break;
+		CASE_TEST(hex_0_alt);    EXPECT_VFPRINTF(0, "|0|0000|   00|", "|%#x|%#04x|%#5.2x|", 0, 0, 0); break;
+		CASE_TEST(dec_alt);      EXPECT_VFPRINTF(0, "|1|0001|   02|", "|%#d|%#04d|%#5.2d|", 1, 1, 2); break; /* '#' is ignored */
 		CASE_TEST(scanf);        EXPECT_ZR(1, test_scanf()); break;
 		CASE_TEST(strerror);     EXPECT_ZR(1, test_strerror()); break;
 		CASE_TEST(printf_error); EXPECT_ZR(1, test_printf_error()); break;
-- 
2.39.5

[PATCH 12/12] selftests/nolibc: Use printf("%.*s", n, "") to align output

[david.laight.linux]

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

Now that printf supports '*' for field widths it can be used to
align the "[OK]" strings in the output.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 21 ++++----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index bf92b9046175..1b0cfd77145a 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -141,21 +141,6 @@ static const char *errorname(int err)
 	}
 }
 
-static void align_result(size_t llen)
-{
-	const size_t align = 64;
-	char buf[align];
-	size_t n;
-
-	if (llen >= align)
-		return;
-
-	n = align - llen;
-	memset(buf, ' ', n);
-	buf[n] = '\0';
-	fputs(buf, stdout);
-}
-
 enum RESULT {
 	OK,
 	FAIL,
@@ -173,8 +158,10 @@ static void result(int llen, enum RESULT r)
 	else
 		msg = " [FAIL]";
 
-	align_result(llen);
-	puts(msg);
+	llen = 64 - llen;
+	if (llen < 0)
+		llen = 0;
+	printf("%*s%s\n", llen, "", msg);
 }
 
 /* The tests below are intended to be used by the macroes, which evaluate
-- 
2.39.5

Re: [PATCH next 00/12] tools/nolibc: Enhance printf()

[David Laight]

On Tue,  3 Feb 2026 10:29:48 +0000
david.laight.linux@gmail.com wrote:

> From: David Laight <david.laight.linux@gmail.com>
> 
> Update printf() so that it handles almost all the non-fp formats.
> In particular:
> - Left alignment.
> - Zero padding.
> - Field precision.
> - Variable field width and precision.
> - Width modifiers q, L, t and z.
> - Conversion specifiers i and X (X generates lower case).
> About the only thing that is missing is octal.
> 
> The tests are updated to match.
> 
> There is a slight increase in code size, but it is minimalised
> by the the heavy use of bit-pattern matches.

I failed to add 'next' to the last two patches, but I don't expect anything
to actually happen until after the next merge window.
If the patch to change all the number->ascii function is committed I'll
add the trivial patch to support octal.

For reference this is the final version:

/* printf(). Supports most of the normal integer and string formats.
 *  - %[#0-+ ][width|*[.precision|*]][{l,t,z,ll,L,j,q}]{d,i,u,c,x,X,p,s,m,%}
 *  - %% generates a single %
 *  - %m outputs strerror(errno).
 *  - # only affects %x and prepends 0x to non-zero values.
 *  - %o (octal) isn't supported.
 *  - %X outputs a..f the same as %x.
 *  - No support for wide characters.
 *  - 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 c;
	int len, written, width, precision;
	unsigned int flags, c_flag;
	char tmpbuf[32 + 24];
	const char *outstr;

	written = 0;
	while (1) {
		outstr = fmt;
		c = *fmt++;
		if (!c)
			break;

		width = 0;
		flags = 0;
		if (c != '%') {
			while (*fmt && *fmt != '%')
				fmt++;
			len = fmt - outstr;
		} else {
			/* we're in a format sequence */

			c = *fmt++;

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

			/* width and precision */
			for (;; c = *fmt++) {
				if (c == '*') {
					precision = va_arg(args, unsigned int);
					c = *fmt++;
				} else {
					for (precision = 0; c >= '0' && c <= '9'; c = *fmt++)
						precision = precision * 10 + (c - '0');
				}
				if (flags & __PF_FLAG('.'))
					break;
				width = precision;
				if (c != '.') {
					/* Default precision for strings */
					precision = INT_MAX;
					break;
				}
				flags |= __PF_FLAG('.');
			}

			/* 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++;
				}
			}

			/* Conversion specifiers are lower case except 'X' treated as 'x' */
			if (!((c >= 'a' && c <= 'z') || (c == 'X' && (c = 'x'))))
				goto bad_conversion_specifier;

			/* Numeric and pointer conversion specifiers.
			 * We need to check for "%p" or "%#x" later, merging here gives better code.
			 * But '#' collides with 'c' so shift right.
			 */
			c_flag = __PF_FLAG(c) | (flags & __PF_FLAG('#')) >> 1;
			if (c_flag & (__PF_FLAG('c') | __PF_FLAG('d') | __PF_FLAG('i') | __PF_FLAG('u') |
					    __PF_FLAG('x') | __PF_FLAG('p') | __PF_FLAG('s'))) {
				unsigned long long v;
				long long signed_v;
				char *out = tmpbuf + 32;
				int sign = 0;

				/* Annoying 'p' === '0' so mask from flags */
				if ((c_flag | (flags & ~__PF_FLAG('p'))) &
				    (__PF_FLAG('p') | __PF_FLAG('s') | __PF_FLAG('l') | __PF_FLAG('t') | __PF_FLAG('z'))) {
					v = va_arg(args, unsigned long);
					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);
					signed_v = (int)v;
				}

				if (c_flag & __PF_FLAG('c')) {
					tmpbuf[0] = v;
					len = 1;
					outstr = tmpbuf;
					goto do_output;
				}

				if (c_flag & __PF_FLAG('s')) {
					if (!v) {
						outstr = "(null)";
						/* Match glibc, nothing output if precision too small */
						len = precision >= 6 ? 6 : 0;
						goto do_output;
					}
					outstr = (void *)v;
do_strnlen_output:
					len = strnlen(outstr, precision);
					goto do_output;
				}

				if (c_flag & (__PF_FLAG('d') | __PF_FLAG('i'))) {
					if (signed_v < 0) {
						sign = '-';
						v = -(signed_v + 1);
						v++;
					} else if (flags & __PF_FLAG('+')) {
						sign = '+';
					} else if (flags & __PF_FLAG(' ')) {
						sign = ' ';
					}
				}

				if (v == 0) {
					/* There are special rules for zero. */
					if (c_flag & __PF_FLAG('p')) {
						/* match glibc, precision is ignored */
						outstr = "(nil)";
						len = 5;
						goto do_output;
					}
					if (!precision) {
						/* Explicit %nn.0d, no digits output */
						len = 0;
						goto prepend_sign;
					}
					/* "#x" should output "0" not "0x0" */
					*out = '0';
					len = 1;
				} else {
					if (c_flag & (__PF_FLAG('d') | __PF_FLAG('i') | __PF_FLAG('u'))) {
						len = u64toa_r(v, out);
					} else {
						len = u64toh_r(v, out);
						if (c_flag & (__PF_FLAG('p') | __PF_FLAG('#' - 1)))
							sign = 'x' | '0' << 8;
					}
				}

				/* Add zero padding */
				if (flags & (__PF_FLAG('0') | __PF_FLAG('.'))) {
					if (!(flags & __PF_FLAG('.'))) {
						if (flags & __PF_FLAG('-'))
							/* Left justify overrides zero pad */
							goto prepend_sign;
						/* Zero pad to field width less sign */
						precision = width;
						if (sign) {
							precision--;
							if (sign >= 256)
								precision--;
						}
					}
					if (precision > 30)
						/* Don't run off the start of tmpbuf[] */
						precision = 30;
					for (; len < precision; len++) {
						/* Stop gcc generating horrid code and memset().
						 * This is OPTIMIZER_HIDE_VAR() from compiler.h.
						 */
						__asm__ volatile("" : "=r"(len) : "0"(len));
						*--out = '0';
					}
				}

prepend_sign:
				for (; sign; sign >>= 8) {
					len++;
					*--out = sign;
				}
				outstr = out;
			}
			else if (c == 'm') {
#ifdef NOLIBC_IGNORE_ERRNO
				outstr = "unknown error";
				len = __builtin_strlen(outstr);
#else
				outstr = strerror(errno);
				goto do_strnlen_output;
#endif /* NOLIBC_IGNORE_ERRNO */
			} else {
bad_conversion_specifier:
				if (c != '%')
					/* Invalid format, output the format string */
					fmt = outstr + 1;
				/* %% is documented as a 'conversion specifier'.
				 * Any flags, precision or length modifier are ignored.
				 */
				outstr = fmt - 1;
				len = 1;
				width = 0;
			}
		}

do_output:
		written += len;

                /* 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 (!flags && cb(state, outstr, len) != 0)
			return -1;
	}

	if (cb(state, NULL, 0) != 0)
		return -1;

	return written;
}
#undef _PF_FLAG

struct __nolibc_fprintf_cb_state {
	FILE *stream;
	unsigned int buf_offset;
	char buf[128];
};

static int __nolibc_fprintf_cb(void *v_state, const char *buf, size_t size)
{
	struct __nolibc_fprintf_cb_state *state = v_state;
	unsigned int off = state->buf_offset;

	if (off + size > sizeof(state->buf) || buf == NULL) {
		state->buf_offset = 0;
		if (off && _fwrite(state->buf, off, state->stream))
			return -1;
		if (size > sizeof(state->buf))
			return _fwrite(buf, size, state->stream);
		off = 0;
	}

	if (size) {
		state->buf_offset = off + size;
		memcpy(state->buf + off, buf, size);
	}
	return 0;
}

static __attribute__((unused, format(printf, 2, 0)))
int vfprintf(FILE *stream, const char *fmt, va_list args)
{
	struct __nolibc_fprintf_cb_state state;

	state.stream = stream;
	state.buf_offset = 0;
	return __nolibc_printf(__nolibc_fprintf_cb, &state, fmt, args);
}

struct __nolibc_sprintf_cb_state {
	char *buf;
	size_t size;
};

static int __nolibc_sprintf_cb(void *v_state, const char *buf, size_t size)
{
	struct __nolibc_sprintf_cb_state *state = v_state;
	char *tgt;

	if (size >= state->size) {
		if (state->size <= 1)
			return 0;
		size = state->size - 1;
	}
	tgt = state->buf;
	if (size) {
		state->size -= size;
		state->buf = tgt + size;
		memcpy(tgt, buf, size);
	} else {
		*tgt = '\0';
	}
	return 0;
}

static __attribute__((unused, format(printf, 3, 0)))
int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
{
	struct __nolibc_sprintf_cb_state state = { .buf = buf, .size = size };

	return __nolibc_printf(__nolibc_sprintf_cb, &state, fmt, args);
}

Re: [PATCH 11/12] selftests/nolibc: Increase coverage of printf format tests

[kernel test robot]

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20260202]

url:    https://github.com/intel-lab-lkp/linux/commits/david-laight-linux-gmail-com/tools-nolibc-printf-Move-length-check-to-snprintf-callback/20260203-183919
base:   next-20260202
patch link:    https://lore.kernel.org/r/20260203103000.20206-12-david.laight.linux%40gmail.com
patch subject: [PATCH 11/12] selftests/nolibc: Increase coverage of printf format tests
config: x86_64-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260203/202602031944.7j98nfRE-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260203/202602031944.7j98nfRE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602031944.7j98nfRE-lkp@intel.com/

All warnings (new ones prefixed by >>):

   nolibc-test.c: In function 'test_dirent':
   nolibc-test.c:784:17: warning: 'readdir_r' is deprecated [-Wdeprecated-declarations]
     784 |                 ret = readdir_r(dir, &dirent, &result);
         |                 ^~~
   In file included from nolibc-test.c:32:
   /usr/include/dirent.h:185:12: note: declared here
     185 | extern int readdir_r (DIR *__restrict __dirp,
         |            ^~~~~~~~~
   nolibc-test.c: At top level:
>> nolibc-test.c:1654:1: warning: 'mat' attribute directive ignored [-Wattributes]
    1654 | {
         | ^
--
>> nolibc-test.c:1654:1: warning: 'mat' attribute directive ignored [-Wattributes]
    1654 | {
         | ^

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 11/12] selftests/nolibc: Increase coverage of printf format tests

[David Laight]

On Tue, 3 Feb 2026 19:22:28 +0100
kernel test robot <lkp@intel.com> wrote:

> Hi,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on next-20260202]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/david-laight-linux-gmail-com/tools-nolibc-printf-Move-length-check-to-snprintf-callback/20260203-183919
> base:   next-20260202
> patch link:    https://lore.kernel.org/r/20260203103000.20206-12-david.laight.linux%40gmail.com
> patch subject: [PATCH 11/12] selftests/nolibc: Increase coverage of printf format tests
> config: x86_64-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260203/202602031944.7j98nfRE-lkp@intel.com/config)
> compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260203/202602031944.7j98nfRE-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202602031944.7j98nfRE-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    nolibc-test.c: In function 'test_dirent':
>    nolibc-test.c:784:17: warning: 'readdir_r' is deprecated [-Wdeprecated-declarations]
>      784 |                 ret = readdir_r(dir, &dirent, &result);
>          |                 ^~~
>    In file included from nolibc-test.c:32:
>    /usr/include/dirent.h:185:12: note: declared here
>      185 | extern int readdir_r (DIR *__restrict __dirp,
>          |            ^~~~~~~~~
>    nolibc-test.c: At top level:
> >> nolibc-test.c:1654:1: warning: 'mat' attribute directive ignored [-Wattributes]  
>     1654 | {
>          | ^
> --
> >> nolibc-test.c:1654:1: warning: 'mat' attribute directive ignored [-Wattributes]  
>     1654 | {
>          | ^
> 

Ah, a line I spotted reading the patches that shouldn't have been there.
Changed it expecting an error when I test-built the next 'round'.
For some reason it didn't fail for me (maybe it needs W=1?).
The line is just before expect_vfprintf() and I was looking into why the
tests weren't getting printf format errors and then thinking I could stop
gcc bleating about a missing format attribute - but that doesn't work
because the code is testing invalid formats.

	David

Re: [PATCH 11/12] selftests/nolibc: Increase coverage of printf format tests

[kernel test robot]

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20260202]

url:    https://github.com/intel-lab-lkp/linux/commits/david-laight-linux-gmail-com/tools-nolibc-printf-Move-length-check-to-snprintf-callback/20260203-183919
base:   next-20260202
patch link:    https://lore.kernel.org/r/20260203103000.20206-12-david.laight.linux%40gmail.com
patch subject: [PATCH 11/12] selftests/nolibc: Increase coverage of printf format tests
config: s390-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260204/202602040045.wtyCVh2h-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project f43d6834093b19baf79beda8c0337ab020ac5f17)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260204/202602040045.wtyCVh2h-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602040045.wtyCVh2h-lkp@intel.com/

All warnings (new ones prefixed by >>):

   nolibc-test.c:784:9: warning: 'readdir_r' is deprecated [-Wdeprecated-declarations]
     784 |                 ret = readdir_r(dir, &dirent, &result);
         |                       ^
   /usr/include/dirent.h:188:28: note: 'readdir_r' has been explicitly marked deprecated here
     188 |      __nonnull ((1, 2, 3)) __attribute_deprecated__;
         |                            ^
   /usr/include/s390x-linux-gnu/sys/cdefs.h:510:51: note: expanded from macro '__attribute_deprecated__'
     510 | # define __attribute_deprecated__ __attribute__ ((__deprecated__))
         |                                                   ^
   nolibc-test.c:982:24: warning: self-comparison always evaluates to false [-Wtautological-compare]
     982 |         if (st.st_atim.tv_sec != st.st_atime || st.st_atim.tv_nsec > 1000000000)
         |                               ^
   nolibc-test.c:985:24: warning: self-comparison always evaluates to false [-Wtautological-compare]
     985 |         if (st.st_mtim.tv_sec != st.st_mtime || st.st_mtim.tv_nsec > 1000000000)
         |                               ^
   nolibc-test.c:988:24: warning: self-comparison always evaluates to false [-Wtautological-compare]
     988 |         if (st.st_ctim.tv_sec != st.st_ctime || st.st_ctim.tv_nsec > 1000000000)
         |                               ^
   nolibc-test.c:1444:54: warning: null passed to a callee that requires a non-null argument [-Wnonnull]
    1444 |                 CASE_TEST(stat_fault);        EXPECT_SYSER(1, stat(NULL, &stat_buf), -1, EFAULT); break;
         |                                                                    ^~~~
   /opt/cross/clang-f43d683409/lib/clang/22/include/__stddef_null.h:26:14: note: expanded from macro 'NULL'
      26 | #define NULL ((void*)0)
         |              ^~~~~~~~~~
   nolibc-test.c:369:22: note: expanded from macro 'EXPECT_SYSER'
     369 |         EXPECT_SYSER2(cond, expr, expret, experr, 0)
         |                             ^~~~
   nolibc-test.c:366:70: note: expanded from macro 'EXPECT_SYSER2'
     366 |         do { if (!(cond)) result(llen, SKIPPED); else ret += expect_syserr2(expr, expret, experr1, experr2, llen); } while (0)
         |                                                                             ^~~~
>> nolibc-test.c:1652:16: warning: unknown attribute 'mat' ignored [-Wunknown-attributes]
    1652 | __attribute__((mat(printf, 3, 0)))
         |                ^~~~~~~~~~~~~~~~~
   6 warnings generated.
--
>> nolibc-test.c:1652:16: warning: unknown attribute 'mat' ignored [-Wunknown-attributes]
    1652 | __attribute__((mat(printf, 3, 0)))
         |                ^~~~~~~~~~~~~~~~~
   1 warning generated.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH next 06/12] tools/nolibc/printf: Add support for left alignment and %[tzLq]d"

[Willy Tarreau]

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

Re: [PATCH next 06/12] tools/nolibc/printf: Add support for left alignment and %[tzLq]d"

[David Laight]

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

Re: [PATCH next 06/12] tools/nolibc/printf: Add support for left alignment and %[tzLq]d"

[Willy Tarreau]

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

Re: [PATCH next 06/12] tools/nolibc/printf: Add support for left alignment and %[tzLq]d"

[David Laight]

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