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

[Rob Clark]

On Tue, Aug 8, 2017 at 7:55 PM, Alexander Graf <agraf at suse.de> wrote:
>
>
> 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 was more thinking if there was some oddball non-arm arch that u-boot
supported which didn't haven good mainline gcc support and required
something ancient ;-)

For arm/arm64, it seems like we could somehow come up w/ a solution
using a new enough toolchain, given that arm support in gcc has been
good for a long time.. not like the old days where we had to use some
codesourcery build (or figure out how to compile the cs src code drop
ourselves).  A toolchain >= 4.9 for arm/arm64 shouldn't be hard to
come by.

BR,
-R

> 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