[U-Boot] [OpenQuestion] stdint.h and inttypes.h in U-Boot ?
Masahiro Yamada
yamada.m at jp.panasonic.com
Mon Dec 22 11:30:09 CET 2014
Hi Simon,
On Tue, 16 Dec 2014 21:44:00 -0700
Simon Glass <sjg at chromium.org> wrote:
> Hi Masahiro,
>
> On 15 December 2014 at 18:47, Masahiro YAMADA <yamada.m at jp.panasonic.com> wrote:
> > Simon,
> >
> >
> > 2014-12-02 5:06 GMT+09:00 Simon Glass <sjg at chromium.org>:
> >> Hi Masahiro,
> >>
> >> On 26 November 2014 at 00:45, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> >>> Hi Tom, Simon, other developers,
> >>>
> >>>
> >>>
> >>> Since commit 0d296cc2d3b8 (Provide option to avoid defining a custom version of uintptr_t)
> >>> and commit 4166ecb247a1 (Add some standard headers external code might need),
> >>> I have been wondering if they were right decisions.
> >>>
> >>>
> >>>
> >>> As arch/arm/include/asm/types.h of Linux Kernel says,
> >>> using 'stdint.h' is not feasible.
> >>>
> >>> -------------------------------------->8-------------------------------------
> >>> /*
> >>> * The C99 types uintXX_t that are usually defined in 'stdint.h' are not as
> >>> * unambiguous on ARM as you would expect. For the types below, there is a
> >>> * difference on ARM between GCC built for bare metal ARM, GCC built for glibc
> >>> * and the kernel itself, which results in build errors if you try to build with
> >>> * -ffreestanding and include 'stdint.h' (such as when you include 'arm_neon.h'
> >>> * in order to use NEON intrinsics)
> >>> *
> >>> * As the typedefs for these types in 'stdint.h' are based on builtin defines
> >>> * supplied by GCC, we can tweak these to align with the kernel's idea of those
> >>> * types, so 'linux/types.h' and 'stdint.h' can be safely included from the same
> >>> * source file (provided that -ffreestanding is used).
> >>> *
> >>> * int32_t uint32_t uintptr_t
> >>> * bare metal GCC long unsigned long unsigned int
> >>> * glibc GCC int unsigned int unsigned int
> >>> * kernel int unsigned int unsigned long
> >>> */
> >>> --------------------------------------8<----------------------------------------
> >>>
> >>
> >> To me this doesn't matter. I feel that int32_t should probably be int
> >> on 32-bit ARM, but actually so long as it is consistent then it is
> >> fine if it is long. In fact so long as these types are defined
> >> consistently, everything works.
> >
> > Gabe Black and you broke the consistency.
> > See my explanation below.
> >
> >
> >
> >
> >>>
> >>> Actually, the kernel never includes <stdint.h> except host programs.
> >>>
> >>>
> >>> Commit 0d296cc2d3b8 introduced "USE_STDINT", but it causes type conflicts
> >>> depending on which GCC is used.
> >>>
> >>>
> >>> With ARM bare metal GCC,
> >>>
> >>> yamada at beagle:~/workspace/u-boot$ make omap3_beagle_defconfig all CROSS_COMPILE=arm-none-eabi- USE_STDINT=1
> >>> #
> >>> # configuration written to .config
> >>> #
> >>> #
> >>> # configuration written to spl/.config
> >>> #
> >>> scripts/kconfig/conf --silentoldconfig Kconfig
> >>> scripts/kconfig/conf --silentoldconfig Kconfig
> >>> CHK include/config.h
> >>> GEN include/autoconf.mk
> >>> GEN include/autoconf.mk.dep
> >>> GEN spl/include/autoconf.mk
> >>> CHK include/config/uboot.release
> >>> CHK include/generated/version_autogenerated.h
> >>> CHK include/generated/timestamp_autogenerated.h
> >>> UPD include/generated/timestamp_autogenerated.h
> >>> CC lib/asm-offsets.s
> >>> In file included from /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint.h:5:0,
> >>> from include/compiler.h:117,
> >>> from include/image.h:19,
> >>> from include/common.h:85,
> >>> from lib/asm-offsets.c:15:
> >>> /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint-gcc.h:40:24: error: conflicting types for 'int32_t'
> >>> include/linux/types.h:99:17: note: previous declaration of 'int32_t' was here
> >>> /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint-gcc.h:52:25: error: conflicting types for 'uint32_t'
> >>> include/linux/types.h:105:17: note: previous declaration of 'uint32_t' was here
> >>> make[2]: *** [lib/asm-offsets.s] Error 1
> >>> make[1]: *** [prepare0] Error 2
> >>> make: *** [__build_one_by_one] Error 2
> >>>
> >>>
> >>
> >> While toolchain is this? I don't see this problem - it seems broken.
> >
> > Not broken at all.
> > I downloaded it from Mentor Graphics
> > http://www.mentor.com/embedded-software/codesourcery
> >
> > This is a bare-metal compiler.
> > If you do not understand, you should read arch/arm/include/asm/types.h
> > of Linux Kernel once again.
> >
>
> OK, so perhaps for that compiler you cannot use USE_STDINT=1? What
> does it define for the conflicting type?
It does define "uint32_t" as "unsigned long".
> >
> >
> > Until commit 0d296cc2d, U-Boot has used the hard-coded defines like Linux.
>
> Well it still does, only that it also now has the *option* of using stdint.h.
No.
It is not "Do not worry, it is optional" things.
You have changed code here ard threr for the optional feature.
Commit aac618a32 (ext4: Use inttypes for printf() string)
Commit 19ea4678c (Use int64_t for time types)
Commit 6bf672592 (Use uint64_t instead of u64 in put_dec())
Commit c6da9ae8a (Tidy up data sizes and function comment in display_options)
etc.
> >
> > In my understaing, we should only use ILP32 and LP64 compilers.
> >
> >
> > short int long longlong pointer
> > ILP32 (32bit system) 16 32 32 64 32
> > LP64 (64bit Unix-like system) 16 32 64 64 64
> >
> >
> > Whether it is 32bit or 64bit system, we can hard-code
> >
> > u32/uint32_t as unsigned int
> > u64/uint64_t as unsigned long long
> > uintptr_t as unsigned long
> >
> > We do not need to refer to compiler-provided headers.
> >
> > Moreover, we __should not__ refer to compiler-provided <stdint.h>.
> >
> >
> > Including <stdint.h> means that we use uint32_t defined by the compiler.
> > It is "unsigned int" on some compilers, and "unsigned long" on
> > some compilers.
>
> I don't seem to have that problem, or at least I have not noticed it
> with printf().
You are not trying to see the problem. You should try.
Go to Linaro page and download the bare-metal toolchain.
http://www.linaro.org/downloads/
> >
> > We still have the hard-coded uint32_t define in <linux/types.h>
> > This causes the type conflict error I showed above.
> >
> > OK, we can fix it, but horrible problems still remain.
> >
> > If we depends on <stdint.h>, we do not know if uint32_t is defined
> > as "unsigned int" or "unsigned long".
> >
> > To print out 32bit variables, we would always have to use PRId32.
> > Do you want to modify all the printf() printing 32bit variables
> > just to make them unreadable?
> >
>
> Ick, I have not seen this. Can you given an example of where this happens?
Again, you are not trying to see that.
Fine. Check my series:
http://patchwork.ozlabs.org/patch/423341/
> >
> >
> >>> OK, we can fix "int32_t" and "uint32_t", but it still seems strange to see that
> >>> "uint32_t" is defined as "unsigned long", whereas "u32" is defined as "unsigned int".
> >>>
> >>> If so, must we fix "u32", "s32", ... all the fixed-width typedefs ?
> >>>
> >>> I notice including <linux/types.h> in the U-Boot source tree and
> >>> <stdint.h> provided by your compiler at the same time is a nightmare.
> >>>
> >>> If we lean toward <stdint.h>, we must ban <linux/types.h>,
> >>> but <stdint.h> is not available all the time.
> >>> For example, kernel.org tool-chains do not provide <stdint.h>.
> >>>
> >>> Maybe we can drastically re-write <linux/types.h> and friends
> >>> to resolve the type conflicts, but I do not think we should not drift apart from the kernel
> >>> because we have borrowed many source files from Linux.
> >>
> >> So far I don't see a big problem.
> >
> > Horrible problem!
> >
> >> I feel that, were stdint.h available
> >> earlier, then types.h might not have been written and we would just
> >> use stdint.h. Presumably stdint.h has been created to fix these sorts
> >> of problems, and in fact for new projects, they would not define their
> >> own types.
> >
> > What is missing is how to pass the variable width nicely to printf()/scanf().
> >
> > printf("foo =" PRId32 "\n", foo_32);
> > printf("bar =" PRId64 "\n", bar_64);
> >
> > This is ridiculous. Extremely unreadable.
> >
> >
> > I want something like this:
> > printf("foo = %32x \n", foo_32);
> > printf("bar =" %64x \n", bar_64);
> >
> >
> > If this had been provided, Linux and U-Boot might have adopted <stdint.h>,
> > but in fact, PRI* is an awful workaround.
>
> Agreed it's not very readable but so long as it only applies to 64-bit
> values and only to printf() it doesn't see bad to me. Provided we
> resolve what you have brought up above.
Check my series.
> >
> >> IMO in time the kernel and U-Boot might move to stdint.h, and having
> >> it as an option at the moment helps us understand the issues. It does
> >> not break any builds.
> >
> > I double it.
> > Using <stdint.h> is a misjudge. It will mess up printf() everywhere.
> >
> > Linux Kernel has guideline how to print each variable type.
> > Documentation/printk-formats.txt
> >
> >
> >> I could be wrong though, time will tell. We should keep an eye on it.
> >
> > If we keep wrong code in the code base, someone might continue development
> > base on it, which is also wrong.
> >
> > In order not to lose our time, wrong code should be immediately removed.
>
> How do you explain stdint.h? So many projects use it - it is now part
> of the C99 standard. Has the world gone mad?
It is trade-off and consistency things.
If you make a decision to use <stdint.h>, it must be consistent everywhere.
<stdint.h> gives int{8,16,32,64}_t and uint{8,16,32,64}_t, uintptr_t etc.
We should always use them for fixed-width variable types
and should never use hard-coded ones.
That means we should not use include/linux/types.h and friends
to hard-code u8/u16/u32/u64 etc.
We always have to use PRIxN, PRIdN etc. to avoid printf-related warnings.
For that, compiler-provided <inttypes.h> must be included.
When you start a new project, you can include <stdint.h> and <inttypes.h>
and follow the that rule from the beginning.
You will see horrible things if you try to apply that rule on U-Boot.
Best Regards
Masahiro Yamada
More information about the U-Boot
mailing list