| Message ID | 1410978828-20292-2-git-send-email-jeroen@myspectrum.nl |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
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? 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. 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. I believe the real problem is there are some source files include both system headers and <linux/compiler.h> at the same time. Best Regards Masahiro Yamada
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. > > 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. Regards, Jeroen
Hi Jeroen, On Thu, 18 Sep 2014 11:15:25 +0200 Jeroen Hofstee <jeroen@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@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
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h index 02ae99e..e057bd2 100644 --- a/include/linux/compiler-gcc.h +++ b/include/linux/compiler-gcc.h @@ -64,8 +64,12 @@ #endif #define __deprecated __attribute__((deprecated)) +#ifndef __packed #define __packed __attribute__((packed)) +#endif +#ifndef __weak #define __weak __attribute__((weak)) +#endif /* * it doesn't make sense on ARM (currently the only user of __naked) to trace @@ -91,8 +95,12 @@ * would be. * [...] */ +#ifndef __pure #define __pure __attribute__((pure)) +#endif +#ifndef __aligned #define __aligned(x) __attribute__((aligned(x))) +#endif #define __printf(a, b) __attribute__((format(printf, a, b))) #define __scanf(a, b) __attribute__((format(scanf, a, b))) #define noinline __attribute__((noinline)) @@ -115,4 +123,6 @@ */ #define uninitialized_var(x) x = x +#ifndef __always_inline #define __always_inline inline __attribute__((always_inline)) +#endif
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" Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> Cc: Tom Rini <trini@ti.com> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> --- include/linux/compiler-gcc.h | 10 ++++++++++ 1 file changed, 10 insertions(+)