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

[Alexander Graf]

On 09.08.17 00:39, Rob Clark wrote:
> 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..

I don't understand? We enable EFI_LOADER on all arm/arm64 systems for a 
good reason, so they all get checked by travis. If we break travis, that 
won't do anyone any good.

I do remember however that Tom wanted to set certain compiler versions 
as minimum required versions. Tom, do you remember which one that was?


Alex