[U-Boot] [PATCH 1/3] compiler_gcc: do not redefine __gnu_attributes

[Masahiro Yamada]

Hi Jeroen,


On Thu, 18 Sep 2014 11:15:25 +0200
Jeroen Hofstee <jeroen at myspectrum.nl> wrote:

> Hello Masahiro,
> 
> On 18-09-14 04:13, Masahiro Yamada wrote:
> > Jeroen,
> >
> >> commit fb8ffd7cfc68b3dc44e182356a207d784cb30b34 "compiler*.h:
> >> sync include/linux/compiler*.h with Linux 3.16" undid the changes
> >> of 7ea50d52849fe8ffa5b5b74c979b60b1045d6fc9 "compiler_gcc: do
> >> not redefine __gnu_attributes". Add the checks back whether these
> >> macro's are already defined (as it causes a lot of noise on e.g.
> >> FreeBSD where these defines are already in cdefs.h)
> >>
> >> As the original patch this checkpatch warning is ignored:
> >> "WARNING: Adding new packed members is to be done with care"
> >
> > Strange.
> >
> > Which source files include cdefs.h?
> The host std* include this file, not a source file.
> 
> > For building u-boot images, sources should
> > only include headers in the u-boot source tree.
> > The standard system headers should not be used
> > except only a few files such as <stdarg.h>.
> >
> > This is the same as Linux Kernel.
> 
> Yes, and stdarg.h includes cdefs.h. So each include of common.h
> causes several warnings.

Oh, my dear!
This is really unfortunate.


> >
> > On the contrary, host programs are allowed to use
> > standard system headers such as <stdio.h>, <stdlib.h> etc,
> > where <linux/compiler.h> should not be included.
> >
> >
> > The root cause of warnings is _not_ that
> > __packed and __weak are always defined in compiler-gcc.h.
> 
> This only works properly it u-boot for the target does not depend
> on any host header at all, which as you mentioned is not the case.
> Hence we shouldn't be surprised if they define something in their
> namespace and hence checking if that is done is fine.
> 
> > I believe the real problem is there are some source files include
> > both system headers and <linux/compiler.h> at the same time.
> >
> 
> No it is a header issue only. The only alternative I can think of is a
> custom version of stdarg etc and I don't really like that idea.

Me neither.


Thanks for explaining this and fully understood.

Acked-by: Masahiro Yamada <yamada.m at jp.panasonic.com>


(Ideally, I hope a little more comments
because I did not know <stdarg.h> on FreeBSD defines __packed, __weak etc.)




Best Regards
Masahiro Yamada