Re: [U-Boot] [RFC][PATCH] Code Clean-up (weak functions)
Joakim Tjernlund wrote:
- There is no purely weak functions and therfore no longer code like: if (do_something) do_somthing(); 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")));
Curious as to why you removed such code? Was it because it didn't work? If so I might have an answer for that. See my post: "Re: [U-Boot] [PATCH V4] cmd_bdinfo: move implementation to arch instead of common"
Jocke
Good to know. This doc also helps:
http://docs.sun.com/app/docs/doc/817-1984/chapter2-11?l=en&a=view
Then, - we must try to not leave undefined weak symbols at all, - or check the symbol before invocation for the safety in case of NULL dereference.
Hello All,
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")));
Good to know. This doc also helps: http://docs.sun.com/app/docs/doc/817-1984/chapter2-11?l=en&a=view Then,
- we must try to not leave undefined weak symbols at all,
- or check the symbol before invocation for the safety in case of NULL
dereference.
My 2 cents: And here an example from the linux kernel that just does the first option:
Create a default fallback routine that can be used if there is no strong implementation: ------------------------------------------------------------------------------------------- __attribute__((weak)) unsigned long long printk_clock(void) { return sched_clock(); } ------------------------------------------------------------------------------------------- and here is an example of the strong implementation (from the ARM architecture): ------------------------------------------------------------------------------------------- unsigned long long printk_clock(void) { return (unsigned long long)(jiffies - INITIAL_JIFFIES) * (1000000000 / HZ); } -------------------------------------------------------------------------------------------
If the strong implementation is available, the weak is simply discarded during linking, if the strong is omitted, the weak is used as fallback. This is a clean, lean and mean example without complex/superfluous aliases or checks for NULL pointers. It should not get any harder than this...
Kind Regards,
Remy
Remy Bohmer wrote:
Create a default fallback routine that can be used if there is no strong implementation:
__attribute__((weak)) unsigned long long printk_clock(void) { return sched_clock(); }
and here is an example of the strong implementation (from the ARM architecture):
unsigned long long printk_clock(void) { return (unsigned long long)(jiffies - INITIAL_JIFFIES) * (1000000000 / HZ); }
If the strong implementation is available, the weak is simply discarded during linking, if the strong is omitted, the weak is used as fallback. This is a clean, lean and mean example without complex/superfluous aliases or checks for NULL pointers. It should not get any harder than this...
+1. I like this one, and U-Boot/MIPS _machine_restart() is implemented in the same manner.
On Thu, Dec 25, 2008 at 11:56 AM, Shinya Kuribayashi shinya.kuribayashi@necel.com wrote:
Remy Bohmer wrote:
Create a default fallback routine that can be used if there is no strong implementation:
__attribute__((weak)) unsigned long long printk_clock(void) { return sched_clock(); }
and here is an example of the strong implementation (from the ARM architecture):
unsigned long long printk_clock(void) { return (unsigned long long)(jiffies - INITIAL_JIFFIES) * (1000000000 / HZ); }
If the strong implementation is available, the weak is simply discarded during linking, if the strong is omitted, the weak is used as fallback. This is a clean, lean and mean example without complex/superfluous aliases or checks for NULL pointers. It should not get any harder than this...
+1. I like this one, and U-Boot/MIPS _machine_restart() is implemented in the same manner.
Looks like removing the aliases seems to be gaining group consensus. Shall I create a patch which removes all the aliases (It will need thorough regression testing)
-----Original Message----- From: Shinya Kuribayashi [mailto:skuribay@ruby.dti.ne.jp] Sent: den 24 december 2008 17:55 To: Joakim Tjernlund Cc: 'Shinya Kuribayashi'; 'Graeme Russ'; u-boot@lists.denx.de Subject: Re: [U-Boot] [RFC][PATCH] Code Clean-up (weak functions)
Joakim Tjernlund wrote:
- There is no purely weak functions and therfore no longer code like: if (do_something) do_somthing(); 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")));
Curious as to why you removed such code? Was it because it didn't work? If so I might have an answer for that. See my post: "Re: [U-Boot] [PATCH V4] cmd_bdinfo: move implementation to arch instead of common"
Jocke
Good to know. This doc also helps:
http://docs.sun.com/app/docs/doc/817-1984/chapter2-11?l=en&a=view
Then,
- we must try to not leave undefined weak symbols at all,
- or check the symbol before invocation for the safety in case of NULL dereference.
Checking for NULL before invocation won't work unless you fix the relocation routine not to relocate NULL values. That is what I have been trying to say several times now. That fix should be performed regardless so no nasty surprises will happen in the future. Perhaps there are other cases besides weak symbols too. You can either fix the assembler routine in all start.S or make all relevant boards use the C-versions I posted.
Jocke
participants (5)
-
Graeme Russ -
Joakim Tjernlund -
Remy Bohmer -
Shinya Kuribayashi -
Shinya Kuribayashi