Remy Bohmer wrote:
Hello Graeme,
2008/12/13 Graeme Russ graeme.russ@gmail.com:
This patch makes all definitions, declarations and usages of weak functions consistent.
Signed-off-by: Graeme Russ graeme.russ@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