On 08/09/2017 12:44 AM, Rob Clark wrote:
On Tue, Aug 8, 2017 at 6:03 PM, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 08/04/2017 09:31 PM, Rob Clark wrote:
This is convenient for efi_loader which deals a lot with utf16.
Signed-off-by: Rob Clark robdclark@gmail.com
Please, put this patch together with [PATCH] vsprintf.c: add GUID printing https://patchwork.ozlabs.org/patch/798362/ and [PATCH v0 06/20] common: add some utf16 handling helpers https://patchwork.ozlabs.org/patch/797968/ into a separate patch series.
These three patches can be reviewed independently of the efi_loader patches and probably will not be integrated via the efi-next tree.
I'll resend these as a separate patchset, and just not in next revision of efi_loader patchset that it is a dependency
lib/vsprintf.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 874a2951f7..0c40f852ce 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -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>";Please, do not use the L-notation here as it requires -fshort-wchar. As we currently cannot switch the complete project to C11 you cannot use the u-notation either.
current plan was to either switch whole project to -fshort-wchar or c11 and rework these patches (as well as a few patches in the efi_loader patchset). (In the c11 case, I'm not sure what we'll use as the fmt string, since afaict that isn't specified. We could use %S although that seems to be a deprecated way to do %ls, or something different like %A, I guess)..
how far are we from c11? If there is stuff I can do to help let me know. If feasible, I'd rather do that first rather than have a bunch of stuff in vsprintf and elsewhere that needs to be cleaned up later after the switch.
buildman downloads very old compilers (gcc < 4.8) from kernel.org which do not support C11. Travis CI uses Ubuntu 14.04 with gcc 4.8.4 which incorrectly throws an error for disk/part.c in C11 mode.
To get things right we would have to * build our own cross tool chains based on a current gcc version * use our own tool chain in Travis for x86-64 or use a docker container with a current gcc version.
In the long run heading for C11 would be the right thing to do. Until then use an initializer { '<', 'N', 'U', 'L', 'L', '>' }. It looks ugly but does not consume more bytes once compiled.
Regards
Heinrich
int len = utf16_strnlen(str, precision);u8 utf8[len * MAX_UTF8_PER_UTF16];Didn't you forget 1 byte for \0 here?
This is what strlnlen does:
The strnlen() function returns the number of characters in the string pointed to by s, **excluding** the terminating null byte ('\0'), but at most maxlen.
I would expect the exclusion of the terminating null word by an utf16_strnlen function.
you are right, but fixing the wrong problem.. the code is definitely wrong since length of a utf16 string != length of a utf8 string, and we don't need to append a null terminator.. so my logic below using 'len' is wrong. I'll fix that in the next version.
int i;*utf16_to_utf8(utf8, str, len) = '\0';if (!(flags & LEFT))while (len < field_width--)ADDCH(buf, ' ');for (i = 0; i < len; ++i)ADDCH(buf, utf8[i]);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 +549,13 @@ repeat: continue;
case 's':
str = string(str, end, va_arg(args, char *),field_width, precision, flags);
if (qualifier == 'l') {%ls refers to wchar with implementation dependent width in the C standard. There is no qualifier for 16-bit wchar. Couldn't we use %us here in reference to the u-notation ( u'MyString' ). This would leave the path open for a standard compliant '%ls'.
hmm, yeah, that is a clever idea, and I like it better than %A or %S.. so if we go the c11 route I'll do that. The c11 committee should have thought of that ;-)
BR, -R
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':