[U-Boot] [PATCH 3/9] common: include <inttypes.h> always

[Masahiro Yamada]

2018-08-04 0:01 GMT+09:00 Tom Rini <trini at konsulko.com>:
> On Thu, Aug 02, 2018 at 10:56:11AM -0600, Simon Glass wrote:
>> +Tom
>>
>> Hi Masahiro,
>>
>> On 30 July 2018 at 19:41, Masahiro Yamada <yamada.masahiro at socionext.com> wrote:
>> > 2018-07-26 22:59 GMT+09:00 Philipp Tomsich
>> > <philipp.tomsich at theobroma-systems.com>:
>> >> With the ram-size variable changed to u64, we'll need appropriate
>> >> macros for printing u64 values correctly either in 32bit builds
>> >> (i.e. ILP32 models) or in 64bit builds (i.e. LP64 models).  Best to
>> >> make the PRIx64 macro available everywhere.
>> >>
>> >> This include inttypes.h from common.h to make the various macros for
>> >> formatted printing available to everyone.
>> >>
>> >> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>> >> ---
>> >
>> >
>> > NACK.
>> >
>> >
>> > PRIx64 is evil crap. I would make the code super ugly.
>> > Do not use it.
>> >
>> >
>> > The right thing to do is use the same typedefs
>> > for all architectures.
>> >
>> > typedef unsigned char       u8;
>> > typedef unsigned short      u16;
>> > typedef unsigned int        u32;
>> > typedef unsigned long long  u64;
>> >
>> > This works for both ILP32 and LP64.
>> >
>> >
>> > Use '%llx' for printing u64 variables _always_.
>> >
>> >
>> >
>> > This is what Linux exactly does.
>> >
>> >
>> >
>> > In fact, Linux defined fixed-width types differently
>> > for different architectures in old days.
>> >
>> >
>> > After long time effort, Linux unified
>> > the fixed-width types for the kernel space.
>> >
>> > https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/int-ll64.h
>> >
>> >
>> >
>> > See Linux commit:
>> >
>> > commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf
>> > Author: Geert Uytterhoeven <geert at linux-m68k.org>
>> > Date:   Thu Jan 23 15:53:43 2014 -0800
>> >
>> >     asm/types.h: Remove include/asm-generic/int-l64.h
>> >
>> >
>> >
>> >
>> >
>> > And, I fixed ARM Trusted Firmware in the same way:
>> >
>> > https://github.com/ARM-software/arm-trusted-firmware/commit/0a2d5b43c81ed6132761023bf43755f13122ddf0
>> >
>> >
>> >
>> >
>> >
>> > U-Boot is still doing wrong,
>> > and core developers in U-Boot do not understand this, unfortunately.
>>
>> While this works in many cases we do seem to have problems with some
>> toolchains. Perhaps things are better now as my problems were a a few
>> years back.


Please pin-point the pre-built toolchains with which
you had problems.


In other words, please show a compiler that has:

  sizeof(int) != 4

        or

  sizeof(long long) != 8




>> Things like size_t with %z caused problems too. I remember
>> m68k producing warnings when I tried this.


This is true, but
the typedefs of uint{8,16,32,64}_t
and the typedef of size_t are
_different_ problems.

Please do not mix them up to exaggerate things.


Most 32 bit architectures use "unsigned int" size_t,
and all 64 bit architectures use "unsigned long" size_t
as stated in
https://github.com/torvalds/linux/blob/v4.17/include/uapi/asm-generic/posix_types.h#L63


Linux kernel hard-codes size_t and ssize_t, thus
people are supposed to use a compiler with proper size_t define.

Such compilers are provided in
https://mirrors.edge.kernel.org/pub/tools/crosstool/



If you are not happy about hard-coding size_t,
you can do like this:


typedef __SIZE_TYPE__ size_t;

typedef __typeof(__builtin_choose_expr(__builtin_types_compatible_p(size_t,
unsigned long), (long)0, (int)0)) ssize_t;




>> I am certainly interested in converting over to this other approach. I
>> am also OK with the PRi stuff, since it only affects a relatively
>> small number of cases.

Whether the code is right or not
should not be judged by the number of use cases.




> It would certainly be worth giving things another try with current
> compilers.
>
> --
> Tom
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>



-- 
Best Regards
Masahiro Yamada