Message ID | 1410978828-20292-4-git-send-email-jeroen@myspectrum.nl |
---|---|
State | Accepted |
Delegated to: | Tom Rini |
Headers | show |
Jeroen, > Since clang has a different definition for uninitialized_var > it will complain that it is redefined in include/compiler.h. > Since these are already defined in linux/compiler.h just remove > this instance. > > Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> > Cc: Tom Rini <trini@ti.com> > Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> I don't mind this patch but it has made me realize another problem. We have both include/compiler.h and include/linux/compiler.h. Some sources use tha former and others use the latter. I don't know how to use the right one in the right place. Header file policy is one of the biggest problem in U-boot. Everyone has added ugly work-arounds to solve his own problem without correct views or judgement. > diff --git a/include/compiler.h b/include/compiler.h > index 9afc11b..1451916 100644 > --- a/include/compiler.h > +++ b/include/compiler.h > @@ -129,9 +129,6 @@ typedef unsigned long int uintptr_t; > > #endif /* USE_HOSTCC */ > > -/* compiler options */ > -#define uninitialized_var(x) x = x > - > #define likely(x) __builtin_expect(!!(x), 1) > #define unlikely(x) __builtin_expect(!!(x), 0) > I am not sure if likely(x) and unlikely(x) should also duplicated here. Best Regards Masahiro Yamada
Hello Masahiro, On 18-09-14 04:14, Masahiro Yamada wrote: >> Since clang has a different definition for uninitialized_var >> it will complain that it is redefined in include/compiler.h. >> Since these are already defined in linux/compiler.h just remove >> this instance. >> >> Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> >> Cc: Tom Rini <trini@ti.com> >> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> > > > I don't mind this patch but it has made me realize > another problem. > > > We have both include/compiler.h and include/linux/compiler.h. > Some sources use tha former and others use the latter. > > I don't know how to use the right one in the right place. no me neither, although it seems arch / drivers tend to use linux/compiler.h more while tools include compiler.h more. > Header file policy is one of the biggest problem in U-boot. > > Everyone has added ugly work-arounds to solve his own problem > without correct views or judgement. > > >> diff --git a/include/compiler.h b/include/compiler.h >> index 9afc11b..1451916 100644 >> --- a/include/compiler.h >> +++ b/include/compiler.h >> @@ -129,9 +129,6 @@ typedef unsigned long int uintptr_t; >> >> #endif /* USE_HOSTCC */ >> >> -/* compiler options */ >> -#define uninitialized_var(x) x = x >> - >> #define likely(x) __builtin_expect(!!(x), 1) >> #define unlikely(x) __builtin_expect(!!(x), 0) >> > > I am not sure if likely(x) and unlikely(x) should also > duplicated here. > yup I wondered about that too. likely / unlikely are used a lot more though then the isolated use of uninitialized_var. And as they don't cause any problems (the definitions are the same) I let them alone, but I think they should be removed as well some day. Regards, Jeroen
On Thu, Sep 18, 2014 at 11:39:44AM +0200, Jeroen Hofstee wrote: > Hello Masahiro, > > On 18-09-14 04:14, Masahiro Yamada wrote: > >>Since clang has a different definition for uninitialized_var > >>it will complain that it is redefined in include/compiler.h. > >>Since these are already defined in linux/compiler.h just remove > >>this instance. > >> > >>Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> > >>Cc: Tom Rini <trini@ti.com> > >>Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> > > > > > >I don't mind this patch but it has made me realize > >another problem. > > > > > >We have both include/compiler.h and include/linux/compiler.h. > >Some sources use tha former and others use the latter. > > > >I don't know how to use the right one in the right place. > > no me neither, although it seems arch / drivers tend to use > linux/compiler.h more while tools include compiler.h more. My first guess is that we can't as easily throw <linux/compiler.h> into tools and thus need that around just for tools. Perhaps we should note as much in <compiler.h> and fix regular code to use <linux/compiler.h> ?
On Wed, Sep 17, 2014 at 08:33:48PM +0200, Jeroen Hofstee wrote: > Since clang has a different definition for uninitialized_var > it will complain that it is redefined in include/compiler.h. > Since these are already defined in linux/compiler.h just remove > this instance. > > Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> > Cc: Tom Rini <trini@ti.com> > Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> Applied to u-boot/master, thanks!
diff --git a/include/compiler.h b/include/compiler.h index 9afc11b..1451916 100644 --- a/include/compiler.h +++ b/include/compiler.h @@ -129,9 +129,6 @@ typedef unsigned long int uintptr_t; #endif /* USE_HOSTCC */ -/* compiler options */ -#define uninitialized_var(x) x = x - #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0)
Since clang has a different definition for uninitialized_var it will complain that it is redefined in include/compiler.h. Since these are already defined in linux/compiler.h just remove this instance. Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> Cc: Tom Rini <trini@ti.com> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl> --- This is only used in ubifs related code and MAKEALL for arm is fine (besides the 5 boards with build issues) --- include/compiler.h | 3 --- 1 file changed, 3 deletions(-)