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

[Rob Clark]

On Tue, Aug 8, 2017 at 8:14 PM, Rob Clark <robdclark at gmail.com> wrote:
> 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.
>

btw, I haven't confirmed it yet (I don't have such an old compiler
handy) but I *think* according to [1] that gcc 4.7 should be new
enough for u"string" literals.. which is kind of the main thing we
want at this point.

that sets the compiler version dependency bar *pretty* low..

[1] https://gcc.gnu.org/gcc-4.7/changes.html

BR,
-R