diff mbox

[U-Boot,3/3] compiler.h: remove duplicated uninitialized_var

Message ID 1410978828-20292-4-git-send-email-jeroen@myspectrum.nl
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Jeroen Hofstee Sept. 17, 2014, 6:33 p.m. UTC
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(-)

Comments

Masahiro Yamada Sept. 18, 2014, 2:14 a.m. UTC | #1
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
Jeroen Hofstee Sept. 18, 2014, 9:39 a.m. UTC | #2
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
Tom Rini Sept. 18, 2014, 9:59 p.m. UTC | #3
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> ?
Tom Rini Sept. 25, 2014, 2:46 p.m. UTC | #4
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 mbox

Patch

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)