All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] env: add ENV_IS_ANYWHERE
@ 2017-07-31 12:42 Rob Clark
  2017-07-31 12:42 ` [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support Rob Clark
  2017-08-03  0:48 ` [U-Boot] [PATCH] env: add ENV_IS_ANYWHERE Tom Rini
  0 siblings, 2 replies; 9+ messages in thread
From: Rob Clark @ 2017-07-31 12:42 UTC (permalink / raw)
  To: u-boot

Useful for devices which would otherwise have to use ENV_IS_NOWHERE.
Tries to find and load uboot.env from any block device (which may be,
for example, hot-pluggable sd-card, SATA or USB disk, so the exact
location is not known at the time u-boot is compiled).

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 cmd/bootefi.c         |   5 ++
 cmd/nvedit.c          |   6 +-
 cmd/usb.c             |   4 +-
 common/Kconfig        |  11 ++++
 common/Makefile       |   1 +
 common/board_r.c      |  11 ++++
 common/env_anywhere.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |   5 ++
 8 files changed, 196 insertions(+), 3 deletions(-)
 create mode 100644 common/env_anywhere.c

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 765383bc95..80611e1412 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -10,6 +10,7 @@
 #include <command.h>
 #include <dm.h>
 #include <efi_loader.h>
+#include <environment.h>
 #include <errno.h>
 #include <libfdt.h>
 #include <libfdt_env.h>
@@ -318,6 +319,10 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 		part = parse_partnum(devnr);
 
 		bootefi_device_path = efi_dp_from_part(desc, part);
+
+#ifdef CONFIG_ENV_IS_ANYWHERE
+		env_set_location(desc, part);
+#endif
 	} else {
 #ifdef CONFIG_NET
 		bootefi_device_path = efi_dp_from_eth();
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index cd17db6409..cab7ed2565 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -53,9 +53,11 @@ DECLARE_GLOBAL_DATA_PTR;
 	!defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
 	!defined(CONFIG_ENV_IS_IN_REMOTE)	&& \
 	!defined(CONFIG_ENV_IS_IN_UBI)		&& \
-	!defined(CONFIG_ENV_IS_NOWHERE)
+	!defined(CONFIG_ENV_IS_NOWHERE)		&& \
+	!defined(CONFIG_ENV_IS_ANYWHERE)
 # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|MMC|FAT|EXT4|\
-NAND|NVRAM|ONENAND|SATA|SPI_FLASH|REMOTE|UBI} or CONFIG_ENV_IS_NOWHERE
+NAND|NVRAM|ONENAND|SATA|SPI_FLASH|REMOTE|UBI} or CONFIG_ENV_IS_NOWHERE or \
+CONFIG_ENV_IS_ANYWHERE
 #endif
 
 /*
diff --git a/cmd/usb.c b/cmd/usb.c
index 4fa456e318..d3347301ca 100644
--- a/cmd/usb.c
+++ b/cmd/usb.c
@@ -23,7 +23,7 @@
 #include <usb.h>
 
 #ifdef CONFIG_USB_STORAGE
-static int usb_stor_curr_dev = -1; /* current device */
+int usb_stor_curr_dev = -1; /* current device */
 #endif
 #if defined(CONFIG_USB_HOST_ETHER) && !defined(CONFIG_DM_ETH)
 static int __maybe_unused usb_ether_curr_dev = -1; /* current ethernet device */
@@ -567,6 +567,7 @@ static void do_usb_start(void)
 {
 	bootstage_mark_name(BOOTSTAGE_ID_USB_START, "usb_start");
 
+#ifndef CONFIG_ENV_IS_ANYWHERE
 	if (usb_init() < 0)
 		return;
 
@@ -575,6 +576,7 @@ static void do_usb_start(void)
 	/* try to recognize storage devices immediately */
 	usb_stor_curr_dev = usb_stor_scan(1);
 # endif
+#endif
 #ifndef CONFIG_DM_USB
 # ifdef CONFIG_USB_KEYBOARD
 	drv_usb_kbd_init();
diff --git a/common/Kconfig b/common/Kconfig
index 361346b092..e5f8b5ed3f 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -210,6 +210,17 @@ config ENV_IS_NOWHERE
 	  Define this if you don't want to or can't have an environment stored
 	  on a storage medium
 
+config ENV_IS_ANYWHERE
+	bool "Environment is on any partition"
+	depends on BLK
+	help
+	  Define this if you want to store the environment on the first
+	  partition found containing /uboot.env.  Environment will be saved
+	  to same partition it was loaded from.  This is suitable for boards
+	  which may be booting from various removable devices (ie. sd-card,
+	  usb-disk, etc) and you don't know@the time u-boot is built what
+	  the boot media will be.
+
 endchoice
 
 config ENV_OFFSET
diff --git a/common/Makefile b/common/Makefile
index 17a92ea2d7..dd5046b78f 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
 obj-$(CONFIG_ENV_IS_IN_REMOTE) += env_remote.o
 obj-$(CONFIG_ENV_IS_IN_UBI) += env_ubi.o
 obj-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
+obj-$(CONFIG_ENV_IS_ANYWHERE) += env_anywhere.o
 
 obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
 obj-$(CONFIG_$(SPL_)OF_LIBFDT) += fdt_support.o
diff --git a/common/board_r.c b/common/board_r.c
index ecca1edb04..1a47f6cf1f 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -693,6 +693,14 @@ static int run_main_loop(void)
 	return 0;
 }
 
+#ifdef CONFIG_ENV_IS_ANYWHERE
+static int env_relocate_late(void)
+{
+	env_relocate_spec_late();
+	return 0;
+}
+#endif
+
 /*
  * Over time we hope to remove these functions with code fragments and
  * stub funtcions, and instead call the relevant function directly.
@@ -898,6 +906,9 @@ static init_fnc_t init_sequence_r[] = {
 #ifdef CONFIG_PS2KBD
 	initr_kbd,
 #endif
+#ifdef CONFIG_ENV_IS_ANYWHERE
+	env_relocate_late,
+#endif
 	run_main_loop,
 };
 
diff --git a/common/env_anywhere.c b/common/env_anywhere.c
new file mode 100644
index 0000000000..dc2fc9e522
--- /dev/null
+++ b/common/env_anywhere.c
@@ -0,0 +1,156 @@
+/*
+ *  Copyright (c) 2017 Rob Clark
+ *
+ *  SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+
+#include <command.h>
+#include <environment.h>
+#include <linux/stddef.h>
+#include <malloc.h>
+#include <memalign.h>
+#include <search.h>
+#include <errno.h>
+#include <part.h>
+#include <blk.h>
+#include <usb.h>
+#include <dm.h>
+#include <fs.h>
+
+#define ENV_FILE "uboot.env"
+
+static char env_name[64] = "ANYWHERE";
+char *env_name_spec = env_name;
+
+extern int usb_stor_curr_dev;
+
+env_t *env_ptr;
+
+static struct blk_desc *env_desc;
+static int env_part;
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int env_init(void)
+{
+	/* use default */
+	gd->env_addr = (ulong)&default_environment[0];
+	gd->env_valid = 1;
+
+	return 0;
+}
+
+#ifdef CONFIG_CMD_SAVEENV
+int saveenv(void)
+{
+	env_t env_new;
+	loff_t size;
+	int err;
+
+	if (!env_desc)
+		return 1;
+
+	err = env_export(&env_new);
+	if (err)
+		return err;
+
+	fs_set_blk_dev2(env_desc, env_part);
+
+	err = fs_write(ENV_FILE, (ulong)&env_new, 0, sizeof(env_t), &size);
+	if (err == -1) {
+		printf("\n** Unable to write \"%s\" to %s **\n",
+		       ENV_FILE, env_name_spec);
+		return 1;
+	}
+
+	puts("done\n");
+	return 0;
+}
+#endif /* CONFIG_CMD_SAVEENV */
+
+static int env_find(void)
+{
+	struct udevice *dev;
+
+#if defined(CONFIG_USB_STORAGE) && defined(CONFIG_DM_USB) && defined(CONFIG_CMD_USB)
+	int err;
+
+	err = usb_init();
+	if (!err)
+		usb_stor_curr_dev = usb_stor_scan(1);
+#endif
+
+	for (uclass_first_device_check(UCLASS_BLK, &dev);
+	     dev;
+	     uclass_next_device_check(&dev)) {
+		struct blk_desc *desc = dev_get_uclass_platdata(dev);
+		disk_partition_t info;
+		int part = 1;
+
+		printf("Scanning disk %s for environment...\n", dev->name);
+
+		/* check all partitions: */
+		while (!part_get_info(desc, part, &info)) {
+			fs_set_blk_dev2(desc, part);
+
+			if (fs_exists(ENV_FILE)) {
+				printf("Found %s on %s:%d\n", ENV_FILE,
+				       dev->name, part);
+
+				snprintf(env_name, sizeof(env_name), "%s:%d",
+					 dev->name, part);
+
+				env_desc = desc;
+				env_part = part;
+
+				return 0;
+			}
+
+			part++;
+		}
+	}
+
+	return 1;
+}
+
+void env_set_location(struct blk_desc *desc, int part)
+{
+	/* if we already have an environment location, keep it: */
+	if (env_desc)
+		return;
+
+	env_desc = desc;
+	env_part = part;
+}
+
+void env_relocate_spec(void)
+{
+	set_default_env(NULL);
+}
+
+void env_relocate_spec_late(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
+	loff_t size;
+	int err;
+
+	if (env_find())
+		goto err_env_relocate;
+
+	fs_set_blk_dev2(env_desc, env_part);
+
+	err = fs_read(ENV_FILE, (ulong)buf, 0, CONFIG_ENV_SIZE, &size);
+	if (err == -1) {
+		printf("\n** Unable to read \"%s\" from %s **\n",
+		       ENV_FILE, env_name_spec);
+		goto err_env_relocate;
+	}
+
+	env_import(buf, 1);
+	return;
+
+err_env_relocate:
+	set_default_env("!could not find environment");
+}
diff --git a/include/environment.h b/include/environment.h
index 6f94986c6b..57f03fe9ad 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -177,6 +177,11 @@ extern env_t *env_ptr;
 extern void env_relocate_spec(void);
 extern unsigned char env_get_char_spec(int);
 
+#ifdef CONFIG_ENV_IS_ANYWHERE
+void env_relocate_spec_late(void);
+void env_set_location(struct blk_desc *desc, int part);
+#endif
+
 #if defined(CONFIG_NEEDS_MANUAL_RELOC)
 extern void env_reloc(void);
 #endif
-- 
2.13.0

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

* [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support
  2017-07-31 12:42 [U-Boot] [PATCH] env: add ENV_IS_ANYWHERE Rob Clark
@ 2017-07-31 12:42 ` Rob Clark
  2017-08-02  2:22   ` Heinrich Schuchardt
  2017-08-03  0:48 ` [U-Boot] [PATCH] env: add ENV_IS_ANYWHERE Tom Rini
  1 sibling, 1 reply; 9+ messages in thread
From: Rob Clark @ 2017-07-31 12:42 UTC (permalink / raw)
  To: u-boot

This is convenient for efi_loader which deals a lot with utf16.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 lib/vsprintf.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 874a2951f7..84e157ecb1 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -270,6 +270,35 @@ static char *string(char *buf, char *end, char *s, int field_width,
 	return buf;
 }
 
+static size_t strnlen16(const u16* s, size_t count)
+{
+	const u16 *sc;
+
+	for (sc = s; count-- && *sc; ++sc)
+		/* nothing */;
+	return sc - s;
+}
+
+static char *string16(char *buf, char *end, u16 *s, int field_width,
+		int precision, int flags)
+{
+	int len, i;
+
+	if (s == NULL)
+		s = L"<NULL>";
+
+	len = strnlen16(s, precision);
+
+	if (!(flags & LEFT))
+		while (len < field_width--)
+			ADDCH(buf, ' ');
+	for (i = 0; i < len; ++i)
+		ADDCH(buf, *s++);
+	while (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 +557,14 @@ repeat:
 			continue;
 
 		case 's':
-			str = string(str, end, va_arg(args, char *),
-				     field_width, precision, flags);
+			if (qualifier == 'l') {
+				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.0

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

* [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support
  2017-07-31 12:42 ` [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support Rob Clark
@ 2017-08-02  2:22   ` Heinrich Schuchardt
  2017-08-02  9:38     ` Rob Clark
  0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2017-08-02  2:22 UTC (permalink / raw)
  To: u-boot

On 07/31/2017 02:42 PM, Rob Clark wrote:
> This is convenient for efi_loader which deals a lot with utf16.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  lib/vsprintf.c | 39 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 874a2951f7..84e157ecb1 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -270,6 +270,35 @@ static char *string(char *buf, char *end, char *s, int field_width,
>  	return buf;
>  }
>  
> +static size_t strnlen16(const u16* s, size_t count)
> +{
> +	const u16 *sc;
> +
> +	for (sc = s; count-- && *sc; ++sc)
> +		/* nothing */;
> +	return sc - s;
> +}
> +
> +static char *string16(char *buf, char *end, u16 *s, int field_width,
> +		int precision, int flags)
> +{
> +	int len, i;
> +
> +	if (s == NULL)
> +		s = L"<NULL>";

The L notation creates a wchar_t string. The width of wchar_t depends on
gcc compiler flag -fshort-wchar.

vsprintf.c is not compiled with -fshort-wchar. So change this to

const u16 null[] = { '<', 'N', 'U', 'L', 'L', '>', 0};
s = null;

> +
> +	len = strnlen16(s, precision);
> +
> +	if (!(flags & LEFT))
> +		while (len < field_width--)
> +			ADDCH(buf, ' ');
> +	for (i = 0; i < len; ++i)
> +		ADDCH(buf, *s++);
> +	while (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 +557,14 @@ repeat:
>  			continue;
>  
>  		case 's':
> -			str = string(str, end, va_arg(args, char *),
> -				     field_width, precision, flags);
> +			if (qualifier == 'l') {

According to ISO 9899:1999 %ls is used to indicate a wchar_t string,
which may be u32 * or u16 * depending on GCC flag -fshort-wchar.

Wouldn't it make sense to use some other notation, e.g. %S, to indicate
that we explicitly mean u16 *?

Please, add a comment into the code indicating why we need u16 * support
referring to the UEFI spec.

Best regards

Heinrich

> +				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':
> 

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

* [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support
  2017-08-02  2:22   ` Heinrich Schuchardt
@ 2017-08-02  9:38     ` Rob Clark
  2017-08-02 17:05       ` Heinrich Schuchardt
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Clark @ 2017-08-02  9:38 UTC (permalink / raw)
  To: u-boot

On Tue, Aug 1, 2017 at 10:22 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 07/31/2017 02:42 PM, Rob Clark wrote:
>> This is convenient for efi_loader which deals a lot with utf16.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  lib/vsprintf.c | 39 +++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 37 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>> index 874a2951f7..84e157ecb1 100644
>> --- a/lib/vsprintf.c
>> +++ b/lib/vsprintf.c
>> @@ -270,6 +270,35 @@ static char *string(char *buf, char *end, char *s, int field_width,
>>       return buf;
>>  }
>>
>> +static size_t strnlen16(const u16* s, size_t count)
>> +{
>> +     const u16 *sc;
>> +
>> +     for (sc = s; count-- && *sc; ++sc)
>> +             /* nothing */;
>> +     return sc - s;
>> +}
>> +
>> +static char *string16(char *buf, char *end, u16 *s, int field_width,
>> +             int precision, int flags)
>> +{
>> +     int len, i;
>> +
>> +     if (s == NULL)
>> +             s = L"<NULL>";
>
> The L notation creates a wchar_t string. The width of wchar_t depends on
> gcc compiler flag -fshort-wchar.
>
> vsprintf.c is not compiled with -fshort-wchar. So change this to
>
> const u16 null[] = { '<', 'N', 'U', 'L', 'L', '>', 0};
> s = null;

oh, I have another patch that adds -fshort-wchar globally.. which I
probably should have split out and sent with this.

The problem is we cannot mix objects using short-wchar and ones that
don't without a compiler warning.  Travis would complain a lot more
but I guess BOOTEFI_HELLO is not normally enabled.

With addition of efi_bootmgr.c we really want to be able to use
L"string" to be u16.. and I don't think u-boot has any good reason to
use 32b wchar.

But maybe for this code I should use wchar_t instead of u16.

BR,
-R

>> +
>> +     len = strnlen16(s, precision);
>> +
>> +     if (!(flags & LEFT))
>> +             while (len < field_width--)
>> +                     ADDCH(buf, ' ');
>> +     for (i = 0; i < len; ++i)
>> +             ADDCH(buf, *s++);
>> +     while (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 +557,14 @@ repeat:
>>                       continue;
>>
>>               case 's':
>> -                     str = string(str, end, va_arg(args, char *),
>> -                                  field_width, precision, flags);
>> +                     if (qualifier == 'l') {
>
> According to ISO 9899:1999 %ls is used to indicate a wchar_t string,
> which may be u32 * or u16 * depending on GCC flag -fshort-wchar.
>
> Wouldn't it make sense to use some other notation, e.g. %S, to indicate
> that we explicitly mean u16 *?
>
> Please, add a comment into the code indicating why we need u16 * support
> referring to the UEFI spec.
>
> Best regards
>
> Heinrich
>
>> +                             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':
>>
>

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

* [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support
  2017-08-02  9:38     ` Rob Clark
@ 2017-08-02 17:05       ` Heinrich Schuchardt
  2017-08-02 18:15         ` Rob Clark
  0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2017-08-02 17:05 UTC (permalink / raw)
  To: u-boot

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 5041 bytes --]

On 08/02/2017 11:38 AM, Rob Clark wrote:
> On Tue, Aug 1, 2017 at 10:22 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 07/31/2017 02:42 PM, Rob Clark wrote:
>>> This is convenient for efi_loader which deals a lot with utf16.
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>  lib/vsprintf.c | 39 +++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 37 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>>> index 874a2951f7..84e157ecb1 100644
>>> --- a/lib/vsprintf.c
>>> +++ b/lib/vsprintf.c
>>> @@ -270,6 +270,35 @@ static char *string(char *buf, char *end, char *s, int field_width,
>>>       return buf;
>>>  }
>>>
>>> +static size_t strnlen16(const u16* s, size_t count)
>>> +{
>>> +     const u16 *sc;
>>> +
>>> +     for (sc = s; count-- && *sc; ++sc)
>>> +             /* nothing */;
>>> +     return sc - s;
>>> +}
>>> +
>>> +static char *string16(char *buf, char *end, u16 *s, int field_width,
>>> +             int precision, int flags)
>>> +{
>>> +     int len, i;
>>> +
>>> +     if (s == NULL)
>>> +             s = L"<NULL>";
>>
>> The L notation creates a wchar_t string. The width of wchar_t depends on
>> gcc compiler flag -fshort-wchar.
>>
>> vsprintf.c is not compiled with -fshort-wchar. So change this to
>>
>> const u16 null[] = { '<', 'N', 'U', 'L', 'L', '>', 0};
>> s = null;
> 
> oh, I have another patch that adds -fshort-wchar globally.. which I
> probably should have split out and sent with this.
> 
> The problem is we cannot mix objects using short-wchar and ones that
> don't without a compiler warning.  Travis would complain a lot more
> but I guess BOOTEFI_HELLO is not normally enabled.
> 
> With addition of efi_bootmgr.c we really want to be able to use
> L"string" to be u16.. and I don't think u-boot has any good reason to
> use 32b wchar.
> 
> But maybe for this code I should use wchar_t instead of u16.
> 
> BR,
> -R

ext4 filenames may contain letters with Unicode values > 2**16,
e.g. using Takri letters: 𑚀𑚁𑚂

So ext4ls probably should be enabled to display these on a Unicode console.

Using -fshort-wchar globally is not necessary. Only UEFI requires 16 bit
wchar_t. We should rather not enforce the UEFI standard on the rest of
the code.

> 
>>> +
>>> +     len = strnlen16(s, precision);
>>> +
>>> +     if (!(flags & LEFT))
>>> +             while (len < field_width--)
>>> +                     ADDCH(buf, ' ');
>>> +     for (i = 0; i < len; ++i)
>>> +             ADDCH(buf, *s++);

I would prefer to see a conversion to UTF-8 here.

Conversion from 32bit Unicode (Or the capped 16bit Unicode of EFI) is
quite easy. This is what I used in another project:

        uint32_t u = s[i];
        char c[5];
        if (u < 0x80) {
            c[0] = u & 0x7F;
            c[1] = 0;
            str.append(c);
        } else if (u < 0x800) {
            c[1] = 0x80 | (u & 0x3F);
            u >>= 6;
            c[0] = 0xC0 | (u & 0x1F);
            c[2] = 0;
            str.append(c);
        } else if (u < 0x10000) {
            c[2] = 0x80 | (u & 0x3F);
            u >>= 6;
            c[1] = 0x80 | (u & 0x3F);
            u >>= 6;
            c[0] = 0xE0 | (u & 0x0F);
            c[3] = 0;
            str.append(c);
        } else if (u < 0x200000) {
            c[3] = 0x80 | (u & 0x3F);
            u >>= 6;
            c[2] = 0x80 | (u & 0x3F);
            u >>= 6;
            c[1] = 0x80 | (u & 0x3F);
            u >>= 6;
            c[0] = 0xF0 | (u & 0x07);
            c[4] = 0;
            str.append(c);
        } else {
            throw invalid;
        }

Best regards

Heinrich

>>> +     while (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 +557,14 @@ repeat:
>>>                       continue;
>>>
>>>               case 's':
>>> -                     str = string(str, end, va_arg(args, char *),
>>> -                                  field_width, precision, flags);
>>> +                     if (qualifier == 'l') {
>>
>> According to ISO 9899:1999 %ls is used to indicate a wchar_t string,
>> which may be u32 * or u16 * depending on GCC flag -fshort-wchar.
>>
>> Wouldn't it make sense to use some other notation, e.g. %S, to indicate
>> that we explicitly mean u16 *?
>>
>> Please, add a comment into the code indicating why we need u16 * support
>> referring to the UEFI spec.
>>
>> Best regards
>>
>> Heinrich
>>
>>> +                             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':
>>>
>>
> 


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

* [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support
  2017-08-02 17:05       ` Heinrich Schuchardt
@ 2017-08-02 18:15         ` Rob Clark
  2017-08-02 21:40           ` Heinrich Schuchardt
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Clark @ 2017-08-02 18:15 UTC (permalink / raw)
  To: u-boot

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 5989 bytes --]

On Wed, Aug 2, 2017 at 1:05 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 08/02/2017 11:38 AM, Rob Clark wrote:
>> On Tue, Aug 1, 2017 at 10:22 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> On 07/31/2017 02:42 PM, Rob Clark wrote:
>>>> This is convenient for efi_loader which deals a lot with utf16.
>>>>
>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>> ---
>>>>  lib/vsprintf.c | 39 +++++++++++++++++++++++++++++++++++++--
>>>>  1 file changed, 37 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>>>> index 874a2951f7..84e157ecb1 100644
>>>> --- a/lib/vsprintf.c
>>>> +++ b/lib/vsprintf.c
>>>> @@ -270,6 +270,35 @@ static char *string(char *buf, char *end, char *s, int field_width,
>>>>       return buf;
>>>>  }
>>>>
>>>> +static size_t strnlen16(const u16* s, size_t count)
>>>> +{
>>>> +     const u16 *sc;
>>>> +
>>>> +     for (sc = s; count-- && *sc; ++sc)
>>>> +             /* nothing */;
>>>> +     return sc - s;
>>>> +}
>>>> +
>>>> +static char *string16(char *buf, char *end, u16 *s, int field_width,
>>>> +             int precision, int flags)
>>>> +{
>>>> +     int len, i;
>>>> +
>>>> +     if (s == NULL)
>>>> +             s = L"<NULL>";
>>>
>>> The L notation creates a wchar_t string. The width of wchar_t depends on
>>> gcc compiler flag -fshort-wchar.
>>>
>>> vsprintf.c is not compiled with -fshort-wchar. So change this to
>>>
>>> const u16 null[] = { '<', 'N', 'U', 'L', 'L', '>', 0};
>>> s = null;
>>
>> oh, I have another patch that adds -fshort-wchar globally.. which I
>> probably should have split out and sent with this.
>>
>> The problem is we cannot mix objects using short-wchar and ones that
>> don't without a compiler warning.  Travis would complain a lot more
>> but I guess BOOTEFI_HELLO is not normally enabled.
>>
>> With addition of efi_bootmgr.c we really want to be able to use
>> L"string" to be u16.. and I don't think u-boot has any good reason to
>> use 32b wchar.
>>
>> But maybe for this code I should use wchar_t instead of u16.
>>
>> BR,
>> -R
>
> ext4 filenames may contain letters with Unicode values > 2**16,
> e.g. using Takri letters: 𑚀𑚁𑚂
>
> So ext4ls probably should be enabled to display these on a Unicode console.
>
> Using -fshort-wchar globally is not necessary. Only UEFI requires 16 bit
> wchar_t. We should rather not enforce the UEFI standard on the rest of
> the code.

The alternative is disabling a gcc warning about mixing 32b and 16b
wchar.. and really mixing 32b and 16b wchar seems like a bad idea.

We could use -fshort-wchar only if EFI_LOADER is enabled.  Technically
if we are a UEFI implementation, we do not need to have ext2/ext4 (or
really anything other than fat/vfat).

>>
>>>> +
>>>> +     len = strnlen16(s, precision);
>>>> +
>>>> +     if (!(flags & LEFT))
>>>> +             while (len < field_width--)
>>>> +                     ADDCH(buf, ' ');
>>>> +     for (i = 0; i < len; ++i)
>>>> +             ADDCH(buf, *s++);
>
> I would prefer to see a conversion to UTF-8 here.
>
> Conversion from 32bit Unicode (Or the capped 16bit Unicode of EFI) is
> quite easy. This is what I used in another project:
>
>         uint32_t u = s[i];
>         char c[5];
>         if (u < 0x80) {
>             c[0] = u & 0x7F;
>             c[1] = 0;
>             str.append(c);
>         } else if (u < 0x800) {
>             c[1] = 0x80 | (u & 0x3F);
>             u >>= 6;
>             c[0] = 0xC0 | (u & 0x1F);
>             c[2] = 0;
>             str.append(c);
>         } else if (u < 0x10000) {
>             c[2] = 0x80 | (u & 0x3F);
>             u >>= 6;
>             c[1] = 0x80 | (u & 0x3F);
>             u >>= 6;
>             c[0] = 0xE0 | (u & 0x0F);
>             c[3] = 0;
>             str.append(c);
>         } else if (u < 0x200000) {
>             c[3] = 0x80 | (u & 0x3F);
>             u >>= 6;
>             c[2] = 0x80 | (u & 0x3F);
>             u >>= 6;
>             c[1] = 0x80 | (u & 0x3F);
>             u >>= 6;
>             c[0] = 0xF0 | (u & 0x07);
>             c[4] = 0;
>             str.append(c);
>         } else {
>             throw invalid;
>         }

I did add a utf16_to_utf8() (based on code from grub) as part of the
efi-variables patch, since there we are dealing with utf16 coming from
outside of grub.  I guess I could use that.  I think that mostly
matters if we end up printing strings that originate outside of
u-boot, but I guess that will be the case for filenames in a
device-path.

BR,
-R

> Best regards
>
> Heinrich
>
>>>> +     while (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 +557,14 @@ repeat:
>>>>                       continue;
>>>>
>>>>               case 's':
>>>> -                     str = string(str, end, va_arg(args, char *),
>>>> -                                  field_width, precision, flags);
>>>> +                     if (qualifier == 'l') {
>>>
>>> According to ISO 9899:1999 %ls is used to indicate a wchar_t string,
>>> which may be u32 * or u16 * depending on GCC flag -fshort-wchar.
>>>
>>> Wouldn't it make sense to use some other notation, e.g. %S, to indicate
>>> that we explicitly mean u16 *?
>>>
>>> Please, add a comment into the code indicating why we need u16 * support
>>> referring to the UEFI spec.
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>> +                             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':
>>>>
>>>
>>
>

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

* [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support
  2017-08-02 18:15         ` Rob Clark
@ 2017-08-02 21:40           ` Heinrich Schuchardt
  2017-08-02 22:25             ` Rob Clark
  0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2017-08-02 21:40 UTC (permalink / raw)
  To: u-boot

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 6521 bytes --]

On 08/02/2017 08:15 PM, Rob Clark wrote:
> On Wed, Aug 2, 2017 at 1:05 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 08/02/2017 11:38 AM, Rob Clark wrote:
>>> On Tue, Aug 1, 2017 at 10:22 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>> On 07/31/2017 02:42 PM, Rob Clark wrote:
>>>>> This is convenient for efi_loader which deals a lot with utf16.
>>>>>
>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>> ---
>>>>>  lib/vsprintf.c | 39 +++++++++++++++++++++++++++++++++++++--
>>>>>  1 file changed, 37 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>>>>> index 874a2951f7..84e157ecb1 100644
>>>>> --- a/lib/vsprintf.c
>>>>> +++ b/lib/vsprintf.c
>>>>> @@ -270,6 +270,35 @@ static char *string(char *buf, char *end, char *s, int field_width,
>>>>>       return buf;
>>>>>  }
>>>>>
>>>>> +static size_t strnlen16(const u16* s, size_t count)
>>>>> +{
>>>>> +     const u16 *sc;
>>>>> +
>>>>> +     for (sc = s; count-- && *sc; ++sc)
>>>>> +             /* nothing */;
>>>>> +     return sc - s;
>>>>> +}
>>>>> +
>>>>> +static char *string16(char *buf, char *end, u16 *s, int field_width,
>>>>> +             int precision, int flags)
>>>>> +{
>>>>> +     int len, i;
>>>>> +
>>>>> +     if (s == NULL)
>>>>> +             s = L"<NULL>";
>>>>
>>>> The L notation creates a wchar_t string. The width of wchar_t depends on
>>>> gcc compiler flag -fshort-wchar.
>>>>
>>>> vsprintf.c is not compiled with -fshort-wchar. So change this to
>>>>
>>>> const u16 null[] = { '<', 'N', 'U', 'L', 'L', '>', 0};
>>>> s = null;
>>>
>>> oh, I have another patch that adds -fshort-wchar globally.. which I
>>> probably should have split out and sent with this.
>>>
>>> The problem is we cannot mix objects using short-wchar and ones that
>>> don't without a compiler warning.  Travis would complain a lot more
>>> but I guess BOOTEFI_HELLO is not normally enabled.
>>>
>>> With addition of efi_bootmgr.c we really want to be able to use
>>> L"string" to be u16.. and I don't think u-boot has any good reason to
>>> use 32b wchar.
>>>
>>> But maybe for this code I should use wchar_t instead of u16.
>>>
>>> BR,
>>> -R
>>
>> ext4 filenames may contain letters with Unicode values > 2**16,
>> e.g. using Takri letters: 𑚀𑚁𑚂
>>
>> So ext4ls probably should be enabled to display these on a Unicode console.
>>
>> Using -fshort-wchar globally is not necessary. Only UEFI requires 16 bit
>> wchar_t. We should rather not enforce the UEFI standard on the rest of
>> the code.
> 
> The alternative is disabling a gcc warning about mixing 32b and 16b
> wchar.. and really mixing 32b and 16b wchar seems like a bad idea.
> 
> We could use -fshort-wchar only if EFI_LOADER is enabled.  Technically
> if we are a UEFI implementation, we do not need to have ext2/ext4 (or
> really anything other than fat/vfat).

You can avoid the problem of variable width wchar by using constants
starting with u (e.g. u"Hello world") which are char16_t (introduced
with C11, #include <uchar.h>) and converting to utf-8 for console output.

This way we do not need -fshort-wchar at all.

Best regards

Heinrich

> 
>>>
>>>>> +
>>>>> +     len = strnlen16(s, precision);
>>>>> +
>>>>> +     if (!(flags & LEFT))
>>>>> +             while (len < field_width--)
>>>>> +                     ADDCH(buf, ' ');
>>>>> +     for (i = 0; i < len; ++i)
>>>>> +             ADDCH(buf, *s++);
>>
>> I would prefer to see a conversion to UTF-8 here.
>>
>> Conversion from 32bit Unicode (Or the capped 16bit Unicode of EFI) is
>> quite easy. This is what I used in another project:
>>
>>         uint32_t u = s[i];
>>         char c[5];
>>         if (u < 0x80) {
>>             c[0] = u & 0x7F;
>>             c[1] = 0;
>>             str.append(c);
>>         } else if (u < 0x800) {
>>             c[1] = 0x80 | (u & 0x3F);
>>             u >>= 6;
>>             c[0] = 0xC0 | (u & 0x1F);
>>             c[2] = 0;
>>             str.append(c);
>>         } else if (u < 0x10000) {
>>             c[2] = 0x80 | (u & 0x3F);
>>             u >>= 6;
>>             c[1] = 0x80 | (u & 0x3F);
>>             u >>= 6;
>>             c[0] = 0xE0 | (u & 0x0F);
>>             c[3] = 0;
>>             str.append(c);
>>         } else if (u < 0x200000) {
>>             c[3] = 0x80 | (u & 0x3F);
>>             u >>= 6;
>>             c[2] = 0x80 | (u & 0x3F);
>>             u >>= 6;
>>             c[1] = 0x80 | (u & 0x3F);
>>             u >>= 6;
>>             c[0] = 0xF0 | (u & 0x07);
>>             c[4] = 0;
>>             str.append(c);
>>         } else {
>>             throw invalid;
>>         }
> 
> I did add a utf16_to_utf8() (based on code from grub) as part of the
> efi-variables patch, since there we are dealing with utf16 coming from
> outside of grub.  I guess I could use that.  I think that mostly
> matters if we end up printing strings that originate outside of
> u-boot, but I guess that will be the case for filenames in a
> device-path.
> 
> BR,
> -R
> 
>> Best regards
>>
>> Heinrich
>>
>>>>> +     while (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 +557,14 @@ repeat:
>>>>>                       continue;
>>>>>
>>>>>               case 's':
>>>>> -                     str = string(str, end, va_arg(args, char *),
>>>>> -                                  field_width, precision, flags);
>>>>> +                     if (qualifier == 'l') {
>>>>
>>>> According to ISO 9899:1999 %ls is used to indicate a wchar_t string,
>>>> which may be u32 * or u16 * depending on GCC flag -fshort-wchar.
>>>>
>>>> Wouldn't it make sense to use some other notation, e.g. %S, to indicate
>>>> that we explicitly mean u16 *?
>>>>
>>>> Please, add a comment into the code indicating why we need u16 * support
>>>> referring to the UEFI spec.
>>>>
>>>> Best regards
>>>>
>>>> Heinrich
>>>>
>>>>> +                             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':
>>>>>
>>>>
>>>
>>
> 


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

* [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support
  2017-08-02 21:40           ` Heinrich Schuchardt
@ 2017-08-02 22:25             ` Rob Clark
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Clark @ 2017-08-02 22:25 UTC (permalink / raw)
  To: u-boot

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 7031 bytes --]

On Wed, Aug 2, 2017 at 5:40 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 08/02/2017 08:15 PM, Rob Clark wrote:
>> On Wed, Aug 2, 2017 at 1:05 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> On 08/02/2017 11:38 AM, Rob Clark wrote:
>>>> On Tue, Aug 1, 2017 at 10:22 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>> On 07/31/2017 02:42 PM, Rob Clark wrote:
>>>>>> This is convenient for efi_loader which deals a lot with utf16.
>>>>>>
>>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>>> ---
>>>>>>  lib/vsprintf.c | 39 +++++++++++++++++++++++++++++++++++++--
>>>>>>  1 file changed, 37 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>>>>>> index 874a2951f7..84e157ecb1 100644
>>>>>> --- a/lib/vsprintf.c
>>>>>> +++ b/lib/vsprintf.c
>>>>>> @@ -270,6 +270,35 @@ static char *string(char *buf, char *end, char *s, int field_width,
>>>>>>       return buf;
>>>>>>  }
>>>>>>
>>>>>> +static size_t strnlen16(const u16* s, size_t count)
>>>>>> +{
>>>>>> +     const u16 *sc;
>>>>>> +
>>>>>> +     for (sc = s; count-- && *sc; ++sc)
>>>>>> +             /* nothing */;
>>>>>> +     return sc - s;
>>>>>> +}
>>>>>> +
>>>>>> +static char *string16(char *buf, char *end, u16 *s, int field_width,
>>>>>> +             int precision, int flags)
>>>>>> +{
>>>>>> +     int len, i;
>>>>>> +
>>>>>> +     if (s == NULL)
>>>>>> +             s = L"<NULL>";
>>>>>
>>>>> The L notation creates a wchar_t string. The width of wchar_t depends on
>>>>> gcc compiler flag -fshort-wchar.
>>>>>
>>>>> vsprintf.c is not compiled with -fshort-wchar. So change this to
>>>>>
>>>>> const u16 null[] = { '<', 'N', 'U', 'L', 'L', '>', 0};
>>>>> s = null;
>>>>
>>>> oh, I have another patch that adds -fshort-wchar globally.. which I
>>>> probably should have split out and sent with this.
>>>>
>>>> The problem is we cannot mix objects using short-wchar and ones that
>>>> don't without a compiler warning.  Travis would complain a lot more
>>>> but I guess BOOTEFI_HELLO is not normally enabled.
>>>>
>>>> With addition of efi_bootmgr.c we really want to be able to use
>>>> L"string" to be u16.. and I don't think u-boot has any good reason to
>>>> use 32b wchar.
>>>>
>>>> But maybe for this code I should use wchar_t instead of u16.
>>>>
>>>> BR,
>>>> -R
>>>
>>> ext4 filenames may contain letters with Unicode values > 2**16,
>>> e.g. using Takri letters: 𑚀𑚁𑚂
>>>
>>> So ext4ls probably should be enabled to display these on a Unicode console.
>>>
>>> Using -fshort-wchar globally is not necessary. Only UEFI requires 16 bit
>>> wchar_t. We should rather not enforce the UEFI standard on the rest of
>>> the code.
>>
>> The alternative is disabling a gcc warning about mixing 32b and 16b
>> wchar.. and really mixing 32b and 16b wchar seems like a bad idea.
>>
>> We could use -fshort-wchar only if EFI_LOADER is enabled.  Technically
>> if we are a UEFI implementation, we do not need to have ext2/ext4 (or
>> really anything other than fat/vfat).
>
> You can avoid the problem of variable width wchar by using constants
> starting with u (e.g. u"Hello world") which are char16_t (introduced
> with C11, #include <uchar.h>) and converting to utf-8 for console output.
>
> This way we do not need -fshort-wchar at all.

oh, that would simplify things a *lot*.. is there a corresponding
printf() modifier?

and any reason to fear requiring c11 (I have none myself)?

If not I'll switch over my patches to using u"string" and fixup efi hello too.

BR,
-R


> Best regards
>
> Heinrich
>
>>
>>>>
>>>>>> +
>>>>>> +     len = strnlen16(s, precision);
>>>>>> +
>>>>>> +     if (!(flags & LEFT))
>>>>>> +             while (len < field_width--)
>>>>>> +                     ADDCH(buf, ' ');
>>>>>> +     for (i = 0; i < len; ++i)
>>>>>> +             ADDCH(buf, *s++);
>>>
>>> I would prefer to see a conversion to UTF-8 here.
>>>
>>> Conversion from 32bit Unicode (Or the capped 16bit Unicode of EFI) is
>>> quite easy. This is what I used in another project:
>>>
>>>         uint32_t u = s[i];
>>>         char c[5];
>>>         if (u < 0x80) {
>>>             c[0] = u & 0x7F;
>>>             c[1] = 0;
>>>             str.append(c);
>>>         } else if (u < 0x800) {
>>>             c[1] = 0x80 | (u & 0x3F);
>>>             u >>= 6;
>>>             c[0] = 0xC0 | (u & 0x1F);
>>>             c[2] = 0;
>>>             str.append(c);
>>>         } else if (u < 0x10000) {
>>>             c[2] = 0x80 | (u & 0x3F);
>>>             u >>= 6;
>>>             c[1] = 0x80 | (u & 0x3F);
>>>             u >>= 6;
>>>             c[0] = 0xE0 | (u & 0x0F);
>>>             c[3] = 0;
>>>             str.append(c);
>>>         } else if (u < 0x200000) {
>>>             c[3] = 0x80 | (u & 0x3F);
>>>             u >>= 6;
>>>             c[2] = 0x80 | (u & 0x3F);
>>>             u >>= 6;
>>>             c[1] = 0x80 | (u & 0x3F);
>>>             u >>= 6;
>>>             c[0] = 0xF0 | (u & 0x07);
>>>             c[4] = 0;
>>>             str.append(c);
>>>         } else {
>>>             throw invalid;
>>>         }
>>
>> I did add a utf16_to_utf8() (based on code from grub) as part of the
>> efi-variables patch, since there we are dealing with utf16 coming from
>> outside of grub.  I guess I could use that.  I think that mostly
>> matters if we end up printing strings that originate outside of
>> u-boot, but I guess that will be the case for filenames in a
>> device-path.
>>
>> BR,
>> -R
>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>>>> +     while (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 +557,14 @@ repeat:
>>>>>>                       continue;
>>>>>>
>>>>>>               case 's':
>>>>>> -                     str = string(str, end, va_arg(args, char *),
>>>>>> -                                  field_width, precision, flags);
>>>>>> +                     if (qualifier == 'l') {
>>>>>
>>>>> According to ISO 9899:1999 %ls is used to indicate a wchar_t string,
>>>>> which may be u32 * or u16 * depending on GCC flag -fshort-wchar.
>>>>>
>>>>> Wouldn't it make sense to use some other notation, e.g. %S, to indicate
>>>>> that we explicitly mean u16 *?
>>>>>
>>>>> Please, add a comment into the code indicating why we need u16 * support
>>>>> referring to the UEFI spec.
>>>>>
>>>>> Best regards
>>>>>
>>>>> Heinrich
>>>>>
>>>>>> +                             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':
>>>>>>
>>>>>
>>>>
>>>
>>
>

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

* [U-Boot] [PATCH] env: add ENV_IS_ANYWHERE
  2017-07-31 12:42 [U-Boot] [PATCH] env: add ENV_IS_ANYWHERE Rob Clark
  2017-07-31 12:42 ` [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support Rob Clark
@ 2017-08-03  0:48 ` Tom Rini
  1 sibling, 0 replies; 9+ messages in thread
From: Tom Rini @ 2017-08-03  0:48 UTC (permalink / raw)
  To: u-boot

On Mon, Jul 31, 2017 at 08:42:01AM -0400, Rob Clark wrote:

> Useful for devices which would otherwise have to use ENV_IS_NOWHERE.
> Tries to find and load uboot.env from any block device (which may be,
> for example, hot-pluggable sd-card, SATA or USB disk, so the exact
> location is not known at the time u-boot is compiled).
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

I can see where this concept is useful.  What I would like to see is
updating the ENV_IS_IN_FAT / ENV_IS_IN_EXT4 / ENV_IS_ANYWHERE so that we
end up with:
- ENV_IS_IN_FILESYSTEM
  - Configured device, part, file.
  - Optional specify device, otherwise help for the option describes the
    logic behind how U-Boot will iterate until it finds something.

-- 
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/20170802/b5d8b565/attachment.sig>

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

end of thread, other threads:[~2017-08-03  0:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-31 12:42 [U-Boot] [PATCH] env: add ENV_IS_ANYWHERE Rob Clark
2017-07-31 12:42 ` [U-Boot] [PATCH] vsprintf.c: add wide string (%ls) support Rob Clark
2017-08-02  2:22   ` Heinrich Schuchardt
2017-08-02  9:38     ` Rob Clark
2017-08-02 17:05       ` Heinrich Schuchardt
2017-08-02 18:15         ` Rob Clark
2017-08-02 21:40           ` Heinrich Schuchardt
2017-08-02 22:25             ` Rob Clark
2017-08-03  0:48 ` [U-Boot] [PATCH] env: add ENV_IS_ANYWHERE Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.