On Tue, Aug 8, 2017 at 8:14 PM, Rob Clark <robdcl...@gmail.com> wrote: > On Tue, Aug 8, 2017 at 7:55 PM, Alexander Graf <ag...@suse.de> wrote: >> >> >> On 09.08.17 00:39, Rob Clark wrote: >>> >>> On Tue, Aug 8, 2017 at 7:08 PM, Heinrich Schuchardt <xypron.g...@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.g...@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 <robdcl...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
