[U-Boot] [U-Boot, v0, 07/20] vsprintf.c: add wide string (%ls) support

[Rob Clark]

On Tue, Aug 8, 2017 at 7:08 PM, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 08/09/2017 12:44 AM, Rob Clark wrote:
>> On Tue, Aug 8, 2017 at 6:03 PM, Heinrich Schuchardt <xypron.glpk at 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 at 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.

ugg, 4.8 is pretty old..   Not sure how much older than 4.8 buildman
uses.  It seems like *some* c11 was supported w/ >=4.6 so if we
approach the conversion piecemeal (for example skipping code that
triggers gcc bugs on old compilers) we might be able to keep 4.8.4
working until travis provides something newer.

(btw, even going back say 8 fedora releases or more, I've used distro
packaged arm and aarch64 toolchains exclusively.. are there that many
distro's where we really can't assume availability of an
cross-toolchain?  If there isn't something newer from kernel.org can
we just drop relying on ancient prebuilt toolchains?  I'm anyways not
hugely a fan of downloading binary executables from even kernel.org,
instead of using something from a distro build system which I at least
know is very locked down.)

> 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.
>

Sure, that I'm less worried about, as much as adding stuff that is
very soon going to be legacy.  Even in vfprintf.c it isn't such a big
deal, as efi_loader where it would be more cumbersome.

Maybe we can write out u"<NULL>" longhand in vsprintf.c as you
suggest, but restrict efi_loader to gcc >= 4.9?  That seems like it
shouldn't be a problem for any arm/arm64 device and it shouldn't be a
problem for any device that is likely to have an efi payload to load
in the first place..

BR,
-R