[PATCH v2 0/3] trivial printf patches

[PATCH v2 0/3] trivial printf patches

[Rasmus Villemoes]

Apparently, improving the QoI of vsnprintf() is too controversial. So
here are the uncontended parts of that series with Simon's R-bs added.

It's not a huge image reduction:

$ ../linux/scripts/bloat-o-meter lib/vsprintf.o.{0,1}
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-218 (-218)
Function                                     old     new   delta
printf                                       220     152     -68
vsnprintf_internal                          2124    1974    -150
Total: Before=4878, After=4660, chg -4.47%

but still worth it.

Rasmus Villemoes (3):
  lib/vsprintf.c: implement printf() in terms of vprintf()
  lib/vsprintf.c: remove stale comment
  lib/vsprintf.c: remove unused ip6_addr_string()

 lib/vsprintf.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

-- 
2.31.1

[PATCH v2 1/3] lib/vsprintf.c: implement printf() in terms of vprintf()

[Rasmus Villemoes]

This saves some code, both in terms of #LOC and .text size, and it is
also the normal convention that foo(...) is implemented in terms of
vfoo().

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 lib/vsprintf.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 9dc96c81c6..cf3982eb03 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -787,22 +787,11 @@ int printf(const char *fmt, ...)
 {
 	va_list args;
 	uint i;
-	char printbuffer[CONFIG_SYS_PBSIZE];
 
 	va_start(args, fmt);
-
-	/*
-	 * For this to work, printbuffer must be larger than
-	 * anything we ever want to print.
-	 */
-	i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args);
+	i = vprintf(fmt, args);
 	va_end(args);
 
-	/* Handle error */
-	if (i <= 0)
-		return i;
-	/* Print the string */
-	puts(printbuffer);
 	return i;
 }
 
-- 
2.31.1

[PATCH v2 2/3] lib/vsprintf.c: remove stale comment

[Rasmus Villemoes]

U-Boot doesn't support %pS/%pF or any other kind of kallsyms-like
lookups. Remove the comment.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 lib/vsprintf.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index cf3982eb03..af0a6e1dcf 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -434,9 +434,6 @@ static char *uuid_string(char *buf, char *end, u8 *addr, int field_width,
  * - 'i' [46] for 'raw' IPv4/IPv6 addresses, IPv6 omits the colons, IPv4 is
  *       currently the same
  *
- * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
- * function pointers are really function descriptors, which contain a
- * pointer to the real address.
  */
 static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		int field_width, int precision, int flags)
-- 
2.31.1

[PATCH v2 3/3] lib/vsprintf.c: remove unused ip6_addr_string()

[Rasmus Villemoes]

There's currently no user of %p[iI]6, so including ip6_addr_string()
in the image is a waste of bytes. It's easy enough to have the
compiler elide it without removing the code completely.

The closest I can find to anybody "handling" ipv6 in U-Boot currently
is in efi_net.c which does

        if (ipv6) {
                ret = EFI_UNSUPPORTED;

As indicated in the comment, it can easily be put back, but preferably
under a config knob.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 lib/vsprintf.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index af0a6e1dcf..c14176dd39 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -434,6 +434,9 @@ static char *uuid_string(char *buf, char *end, u8 *addr, int field_width,
  * - 'i' [46] for 'raw' IPv4/IPv6 addresses, IPv6 omits the colons, IPv4 is
  *       currently the same
  *
+ * Note: IPv6 support is currently if(0)'ed out. If you ever need
+ * %pI6, please add an IPV6 Kconfig knob, make your code select or
+ * depend on that, and change the 0 below to CONFIG_IS_ENABLED(IPV6).
  */
 static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		int field_width, int precision, int flags)
@@ -478,7 +481,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		flags |= SPECIAL;
 		/* Fallthrough */
 	case 'I':
-		if (fmt[1] == '6')
+		/* %pI6 currently unused */
+		if (0 && fmt[1] == '6')
 			return ip6_addr_string(buf, end, ptr, field_width,
 					       precision, flags);
 		if (fmt[1] == '4')
-- 
2.31.1

Re: [PATCH v2 1/3] lib/vsprintf.c: implement printf() in terms of vprintf()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

On Fri, May 28, 2021 at 12:20:44AM +0200, Rasmus Villemoes wrote:

> This saves some code, both in terms of #LOC and .text size, and it is
> also the normal convention that foo(...) is implemented in terms of
> vfoo().
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 2/3] lib/vsprintf.c: remove stale comment

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 334 bytes --]

On Fri, May 28, 2021 at 12:20:45AM +0200, Rasmus Villemoes wrote:

> U-Boot doesn't support %pS/%pF or any other kind of kallsyms-like
> lookups. Remove the comment.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 3/3] lib/vsprintf.c: remove unused ip6_addr_string()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

On Fri, May 28, 2021 at 12:20:46AM +0200, Rasmus Villemoes wrote:

> There's currently no user of %p[iI]6, so including ip6_addr_string()
> in the image is a waste of bytes. It's easy enough to have the
> compiler elide it without removing the code completely.
> 
> The closest I can find to anybody "handling" ipv6 in U-Boot currently
> is in efi_net.c which does
> 
>         if (ipv6) {
>                 ret = EFI_UNSUPPORTED;
> 
> As indicated in the comment, it can easily be put back, but preferably
> under a config knob.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]