vsprintf and short-wchar

This patchset enables -fshort-wchar and adds support to vsprintf (%ls), along with GUID printing. Both things needed by efi_loader.

Unlike the previous version of the patchset, which had a kconfig option selected by efi_loader to enable -fshort-wchar, and at Tom's suggestion, this patchset unconditionally switches to -fshort-wchar.

Note that this patchset and the fs_readdir patchset (which I'll resend today) can be applied in either order. But both are dependencies for the stack of efi_loader patches to make normal distro boot work (shim/ fallback and upstream grub being able to locate it's grub.cfg properly). So either these could go before pulling efi-next or possibly have multiple efi-next pull requests so Heinrich's efi_loader patches can go first.

Rob Clark (5):
kbuild: Enable -fshort-wchar
lib: add some utf16 handling helpers
vsprintf.c: add UTF-16 string (%ls) support
vsprintf.c: add GUID printing
examples: add fallback memcpy

 Makefile                     |   1 +
 examples/api/Makefile        |   2 +
 examples/api/glue.c          |  12 +++++
 include/charset.h            |  65 ++++++++++++++++++++++++++++
 include/config_fallbacks.h   |   1 +
 lib/Makefile                 |   1 +
 lib/charset.c                | 101 +++++++++++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_console.c |  17 ++------
 lib/vsprintf.c               |  76 ++++++++++++++++++++++++++++++--
 9 files changed, 258 insertions(+), 18 deletions(-)
 create mode 100644 include/charset.h
 create mode 100644 lib/charset.c

2.13.5

EFI_LOADER really wants UTF-16 strings (ie. %ls and L"string" are 16bit chars instead of 32bit chars). But rather than enabling -fshort-wchar conditionally if EFI_LOADER is enabled, it was deemed preferrable to globally switch.

---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

index 5bb3b09115..d635de12ab 100644
@@ -360,6 +360,7 @@ KBUILD_CPPFLAGS := -D__KERNEL__ -D__UBOOT__
 KBUILD_CFLAGS   := -Wall -Wstrict-prototypes \
 		   -Wno-format-security \
 		   -fno-builtin -ffreestanding
+KBUILD_CFLAGS	+= -fshort-wchar
 KBUILD_AFLAGS   := -D__ASSEMBLY__
 
 # Read UBOOTRELEASE from include/config/uboot.release (if it exists)

2.13.5

2017-09-09 19:47 GMT+09:00 Rob Clark <[email protected]>:

Best Regards
Masahiro Yamada

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170912/e3c70fa7/attachment.sig>

We'll eventually want these in a few places in efi_loader, and also vsprintf.

---
 include/charset.h            |  65 ++++++++++++++++++++++++++++
 lib/Makefile                 |   1 +
 lib/charset.c                | 101 +++++++++++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_console.c |  17 ++------
 4 files changed, 170 insertions(+), 14 deletions(-)
 create mode 100644 include/charset.h
 create mode 100644 lib/charset.c

new file mode 100644
index 0000000000..39279f746a
@@ -0,0 +1,65 @@
+/*
+ *  charset conversion utils
+ *
+ *  Copyright (c) 2017 Rob Clark
+ *
+ *  SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef __CHARSET_H_
+#define __CHARSET_H_
+
+#define MAX_UTF8_PER_UTF16 4
+
+/**
+ * utf16_strlen() - Get the length of an utf16 string
+ *
+ * Returns the number of 16 bit characters in an utf16 string, not
+ * including the terminating NULL character.
+ *
+ * @in     the string to measure
+ * @return the string length
+ */
+size_t utf16_strlen(const uint16_t *in);
+
+/**
+ * utf16_strnlen() - Get the length of a fixed-size utf16 string.
+ *
+ * Returns the number of 16 bit characters in an utf16 string,
+ * not including the terminating NULL character, but at most
+ * 'count' number of characters.  In doing this, utf16_strnlen()
+ * looks at only the first 'count' characters.
+ *
+ * @in     the string to measure
+ * @count  the maximum number of characters to count
+ * @return the string length, up to a maximum of 'count'
+ */
+size_t utf16_strnlen(const uint16_t *in, size_t count);
+
+/**
+ * utf16_strcpy() - UTF16 equivalent of strcpy()
+ */
+uint16_t *utf16_strcpy(uint16_t *dest, const uint16_t *src);
+
+/**
+ * utf16_strdup() - UTF16 equivalent of strdup()
+ */
+uint16_t *utf16_strdup(const uint16_t *s);
+
+/**
+ * utf16_to_utf8() - Convert an utf16 string to utf8
+ *
+ * Converts 'size' characters of the utf16 string 'src' to utf8
+ * written to the 'dest' buffer.
+ *
+ * NOTE that a single utf16 character can generate up to 4 utf8
+ * characters.  See MAX_UTF8_PER_UTF16.
+ *
+ * @dest   the destination buffer to write the utf8 characters
+ * @src    the source utf16 string
+ * @size   the number of utf16 characters to convert
+ * @return the pointer to the first unwritten byte in 'dest'
+ */
+uint8_t *utf16_to_utf8(uint8_t *dest, const uint16_t *src, size_t size);
+
+#endif /* __CHARSET_H_ */

index 2eef1eb80e..24df11b88f 100644
@@ -19,6 +19,7 @@ obj-$(CONFIG_OF_LIVE) += of_live.o
 obj-$(CONFIG_CMD_DHRYSTONE) += dhry/
 
 obj-$(CONFIG_AES) += aes.o
+obj-y += charset.o
 obj-$(CONFIG_USB_TTY) += circbuf.o
 obj-y += crc7.o
 obj-y += crc8.o

index 5ebce4b544..3fc82b8726 100644
@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <charset.h>
 #include <efi_loader.h>
 
 static bool console_size_queried;

@@ -138,20 +139,8 @@ static efi_status_t EFIAPI efi_cout_reset(
 
 static void print_unicode_in_utf8(u16 c)
 {
-	char utf8[4] = { 0 };
-	char *b = utf8;
-
-	if (c < 0x80) {
-		*(b++) = c;
-	} else if (c < 0x800) {
-		*(b++) = 192 + c / 64;
-		*(b++) = 128 + c % 64;
-	} else {
-		*(b++) = 224 + c / 4096;
-		*(b++) = 128 + c / 64 % 64;
-		*(b++) = 128 + c % 64;
-	}
-
+	char utf8[MAX_UTF8_PER_UTF16] = { 0 };
+	utf16_to_utf8((u8 *)utf8, &c, 1);
 	puts(utf8);
 }
 

2.13.5

UEFI wants UCS-2 (the 16 bit only Windows thing), not UTF16 (the may-also-be-32bit-wide thing).

So I'm not sure how far away this code is from the ultimate truth of what we need for UEFI compatibility. Do you have any gut feeling?

Alex

+ leif & peter

I'm not entirely sure. Grub seems to treat it as UTF-16, and from a 30sec look at wikipedia, it seems like UCS-2 is a subset of UTF-16. But I won't claim to be a unicode expert.

BR,
-R

I agree, but we should make sure to have our naming straight. If we only support the BMP (basic multiplingual plane) part of UTF-16, we should

a) Indicate that in the function names
b) Bark if we get input that's outside the range?

Otherwise sooner or later people will get confused, as UTF-16 strings can for example be shorter than the sum of 16-bit-fields they occupy.

Alex

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170912/851a99cf/attachment.sig>

This is convenient for efi_loader which deals a lot with UTF-16. Only enabled with CC_SHORT_WCHAR, leaving room to add a UTF-32 version when CC_SHORT_WCHAR is not enabled.

---
 examples/api/Makefile |  1 +
 lib/vsprintf.c        | 30 ++++++++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

index dab6398bab..87c15d0f68 100644
@@ -34,6 +34,7 @@ EXT_COBJ-y += lib/div64.o
 EXT_COBJ-y += lib/string.o
 EXT_COBJ-y += lib/time.o
 EXT_COBJ-y += lib/vsprintf.o
+EXT_COBJ-y += lib/charset.o
 EXT_SOBJ-$(CONFIG_PPC) += arch/powerpc/lib/ppcstring.o
 ifeq ($(ARCH),arm)
 EXT_SOBJ-$(CONFIG_USE_ARCH_MEMSET) += arch/arm/lib/memset.o

index 874a2951f7..97bed9d36d 100644
@@ -17,6 +17,7 @@
 #include <linux/ctype.h>
 
 #include <common.h>
+#include <charset.h>
 
 #include <div64.h>
 #define noinline __attribute__((noinline))

@@ -270,6 +271,26 @@ static char *string(char *buf, char *end, char *s, int field_width,
 	return buf;
 }
 
+static char *string16(char *buf, char *end, u16 *s, int field_width,
+		int precision, int flags)
+{
+	u16 *str = s ? s : L"<NULL>";
+	int utf16_len = utf16_strnlen(str, precision);
+	u8 utf8[utf16_len * MAX_UTF8_PER_UTF16];
+	int utf8_len, i;
+
+	utf8_len = utf16_to_utf8(utf8, str, utf16_len) - utf8;
+
+	if (!(flags & LEFT))
+		while (utf8_len < field_width--)
+			ADDCH(buf, ' ');
+	for (i = 0; i < utf8_len; ++i)
+		ADDCH(buf, utf8[i]);
+	while (utf8_len < field_width--)
+		ADDCH(buf, ' ');
+	return buf;
+}
+
 #ifdef CONFIG_CMD_NET
 static const char hex_asc[] = "0123456789abcdef";
 #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]

@@ -528,8 +549,13 @@ repeat:
 			continue;
 
 		case 's':
-			str = string(str, end, va_arg(args, char *),
-				     field_width, precision, flags);
+			if (qualifier == 'l' && !IS_ENABLED(CONFIG_SPL_BUILD)) {
+				str = string16(str, end, va_arg(args, u16 *),
+					       field_width, precision, flags);
+			} else {
+				str = string(str, end, va_arg(args, char *),
+					     field_width, precision, flags);
+			}
 			continue;
 
 		case 'p':

2.13.5

Would like to see simple test cases for these (e.g. somewhere like test/vsprint.c). See test/ut_print.c for an example.

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170912/9a0aa91a/attachment.sig>

This works (roughly) the same way as linux's, but we currently always print lower-case (ie. we just keep %pUB and %pUL for compat with linux), mostly just because that is what uuid_bin_to_str() supports.

%pUb: 01020304-0506-0708-090a-0b0c0d0e0f10
%pUl: 04030201-0605-0807-090a-0b0c0d0e0f10

It will be used by a later efi_loader paths for efi variables and for device-path-to-text protocol, and also quite useful for debug prints of protocol GUIDs.

---
 examples/api/Makefile      |  1 +
 include/config_fallbacks.h |  1 +
 lib/vsprintf.c             | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 46 insertions(+), 2 deletions(-)

index 87c15d0f68..899527267d 100644
@@ -35,6 +35,7 @@ EXT_COBJ-y += lib/string.o
 EXT_COBJ-y += lib/time.o
 EXT_COBJ-y += lib/vsprintf.o
 EXT_COBJ-y += lib/charset.o
+EXT_COBJ-$(CONFIG_LIB_UUID) += lib/uuid.o
 EXT_SOBJ-$(CONFIG_PPC) += arch/powerpc/lib/ppcstring.o
 ifeq ($(ARCH),arm)
 EXT_SOBJ-$(CONFIG_USE_ARCH_MEMSET) += arch/arm/lib/memset.o

index 46b7a2a6f2..2c4d43d672 100644
@@ -58,6 +58,7 @@
 
 #if (CONFIG_IS_ENABLED(PARTITION_UUIDS) || \
 	CONFIG_IS_ENABLED(EFI_PARTITION) || \
+	CONFIG_IS_ENABLED(EFI_LOADER) || \
 	defined(CONFIG_RANDOM_UUID) || \
 	defined(CONFIG_CMD_UUID) || \
 	defined(CONFIG_BOOTP_PXE)) && \

2.13.5

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170912/eaadcf04/attachment.sig>

Solves build issue:

Building current source for 134 boards (12 threads, 1 job per thread)
arm: + lsxhl
+examples/api/vsprintf.o: In function `string16':
+lib/vsprintf.c:278: undefined reference to memcpy' +examples/api/uuid.o: In function uuid_bin_to_str':
+lib/uuid.c:197: undefined reference to memcpy' +lib/uuid.c:199: undefined reference to memcpy'
+make[3]: *** [examples/api/demo] Error 1
+make[2]: *** [examples/api] Error 2
+make[1]: *** [examples] Error 2
+make: *** [sub-make] Error 2

    133    0    1 /134    sheevaplug

---
 examples/api/glue.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

index 8aabf32c89..575c1e55f3 100644
@@ -416,3 +416,15 @@ void ub_display_clear(void)
 {
 	syscall(API_DISPLAY_CLEAR, NULL);
 }
+
+__weak void *memcpy(void *dest, const void *src, size_t size)
+{
+	unsigned char *dptr = dest;
+	const unsigned char *ptr = src;
+	const unsigned char *end = src + size;
+
+	while (ptr < end)
+		*dptr++ = *ptr++;
+
+	return dest;
+}

2.13.5

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170912/9fd0ccb0/attachment.sig>