[U-Boot] [PATCH 1/7] types.h: define (u)int8_t, (u)int16_t, (u)int32_t based on compiler info (MAD)

[Masahiro Yamada]

Hi Simon,


On Mon, 22 Dec 2014 21:41:48 -0700
Simon Glass <sjg at chromium.org> wrote:

> Hi Masahiro,
> 
> On 22 December 2014 at 03:15, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> > The intent of this series is to show the nasty problems introduced
> > by <stdint.h>.
> >
> > Simon and I are already discussing this in the following thread:
> > http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/203954
> > http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/198550/focus=205880
> 
> I'll reply on that thread but I will add a few comments on each patch as well.
> 
> >
> >    -----
> >
> > Commit 0d296cc2d3b8 (Provide option to avoid defining a custom version
> > of uintptr_t.) introduced CONFIG_USE_STDINT.  If it is enabled,
> > <stdint.h> is included and uint64_t, u_int64_t, int64_t are defined
> > based on the compiler-provided information.  While, inconsistently,
> > that commit left uint_{8,16,32}_t, u_int{8,16,32}t, int_{8,16,32}t
> > to be hard-coded in include/linux/types.h.
> >
> > This causes type conflicts between the typedefs defined in the compiler's
> > <stdint.h> and the ones hard-coded in include/linux/types.h.
> >
> > As you may know, 'uint32_t' is defined as 'unsigned long' in some compilers
> > such as bare metal ARM toolchain, whereas it is defined as 'unsigned int'
> > in kernel.org ones.
> > (This is also clearly mentioned in Linux's arch/arm/include/asm/types.h)
> 
> In that file, it actually redefines the types so that stdint.h can be
> included. Are you suggesting that we should follow the same route?


No.
What I wanted you to notice was the type conflicts. This matrix:

 *                    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



Undefing __UINT32_TYPE__ etc. is an ugly workaround.

The best way is to avoid including <stdint.h>.





> That suggests that Gabe's implementation is not correct, but not that
> the whole concept is wrong.
> 
>  * 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).


Accoding to Documentation/arm/kernel_mode_neon.txt,
the kernel deos not want to include <stdint.h>.

It wants to <arm_neon.h> but unfortunately <arm_neon.h> is including <stdint.h>.

That is the difference:  it looks like you like to include <stdint.h>.





> >
> > If you make a decision to use a compiler-provided <stdint.h>,
> > we cannot hard-code the fixed-width types.
> > To avoid the type conflicts, we must make all of them compiler-dependent
> > consistently.
> >
> > You can reproduce this problem, for example, with a Linaro toolchain.
> >
> > Visit "http://www.linaro.org/downloads/" and download "Bare-metal toolchain
> > for Cortex-R/M and Cortex-A".
> >
> > $ make USE_STDINT=1 CROSS_COMPILE=arm-none-eabi- omap4_panda_defconfig all
> >   HOSTCC  scripts/basic/fixdep
> >   HOSTCC  scripts/kconfig/conf.o
> >    [snip]
> >   CC      lib/asm-offsets.s
> > In file included from /opt/gcc-arm-none-eabi-4_7-2013q1/bin/../lib/gcc/arm-none-eabi/4.7.3/include/stdint.h:5:0,
> >                  from include/compiler.h:117,
> >                  from include/image.h:19,
> >                  from include/common.h:82,
> >                  from lib/asm-offsets.c:15:
> > /opt/gcc-arm-none-eabi-4_7-2013q1/bin/../lib/gcc/arm-none-eabi/4.7.3/include/stdint-gcc.h:40:24: error: conflicting types for 'int32_t'
> > In file included from include/common.h:21:0,
> >                  from lib/asm-offsets.c:15:
> > include/linux/types.h:99:17: note: previous declaration of 'int32_t' was here
> > In file included from /opt/gcc-arm-none-eabi-4_7-2013q1/bin/../lib/gcc/arm-none-eabi/4.7.3/include/stdint.h:5:0,
> >                  from include/compiler.h:117,
> >                  from include/image.h:19,
> >                  from include/common.h:82,
> >                  from lib/asm-offsets.c:15:
> > /opt/gcc-arm-none-eabi-4_7-2013q1/bin/../lib/gcc/arm-none-eabi/4.7.3/include/stdint-gcc.h:52:25: error: conflicting types for 'uint32_t'
> > In file included from include/common.h:21:0,
> >                  from lib/asm-offsets.c:15:
> > include/linux/types.h:105:17: note: previous declaration of 'uint32_t' was here
> 
> I suspect I have not tested this toolchain, so have not see this
> problem. Perhaps this compiler cannot support stdint.h?

It does support stdint.h.
It is not the compiler's fault, but your and Gabe's fault.


> In any case I don't think it is necessary to fiddle with the 8, 16,
> 32-bit types, so this patch is not needed.
> 

As I wrote in another reply, please answer:

 Why do you want to change the 64bit-types only?



Best Regards
Masahiro Yamada