[U-Boot] [RFC][PATCH] Code Clean-up (weak functions)

[Graeme Russ]

Remy Bohmer wrote:
> Hello Graeme,
> 
> 2008/12/13 Graeme Russ <graeme.russ at gmail.com>:
>> This patch makes all definitions, declarations and usages of weak functions
>> consistent.
>>
>> Signed-off-by: Graeme Russ <graeme.russ at gmail.com>
> 
> Just curious:
> What is the relation of this patch to the problem discussed earlier:
> http://www.mail-archive.com/u-boot@lists.denx.de/msg05367.html
> Does this patch repair anything, or could it maybe break things?

The problem discussed in the above thread is related to overriding weak
functions not working if the override is the only function in a module (.o)
within a library (.a) - This patch does not alter this behavior

> I ask this because the weak linking seemed not work as many expected,
> and I guess you cannot test all architectures/boards...

This patch is intended to make the usage of weak functions consistent so
that when anyone goes looking in the code for an example to implement their
own code, they will not be presented with 4 or 5 options. This patch
modifies the small handful that were inconsistent on the assumption that
the (majority) worked. For example, this patch should resolve the
show_boot_progress() issue as per:

http://www.mail-archive.com/u-boot@lists.denx.de/msg05998.html


> 
>> - If the weak alias decleration exceeds 80 columns, the __attribute__ is placed
>>  on the following line, indented by one tab
> 
> The benefit on 1 line would be that it is easy recognisable with grep

true - I thought about that but decided against breaking the 80 column rule
- Wolfgang, what are your thoughts on this?

> which instance of a function is actually being used (or in fact should
> be used due to the fuzzy behaviour of weak in U-boot...) without
> having to go through all the code. That benefit is gone when it is
> moved across different lines.

I didn't think about that - good point

> 
> Maybe it is an idea to move the attribute(weak,...) construction to a
> macro, like in Linux:
> #define __weak __attribute__((weak))

Yes, but it would need the alias as well - most that already exceed 80
columns would still with the macro

> 
> In that case it can be changed easier when a non-GCC compiler is being
> used, and would keep the lines shorter, such that it still fits on 1
> line?

Potentially, but we would have to go through and touch all the weak
functions, not just the small subset hit by this patch

> 
>>  All instances have been replaced by empty functions with an alias. e.g.
>>     void __do_something (args) {}
>>     do_something(args) __atttribute__((weak, alias("__do_something")));
> 
> Notice that in Linux, the 'alias' construction is not being used
> massively. Can it be removed here also, or is it somehow mandatory
> here?

I don't think it is mandatory but it is in the majority in u-boot.

> Removing it would keep the lines short as well...
> 
> Kind Regards,
> 
> Remy
> 

Thanks for the comments,

Regards,


Graeme