[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Preboot support
From: |
Yoshinori K. Okuji |
Subject: |
Re: [PATCH] Preboot support |
Date: |
Tue, 14 Apr 2009 23:42:52 +0900 |
User-agent: |
KMail/1.9.10 |
On Monday 13 April 2009 02:19:07 phcoder wrote:
> What about this one?
- ChangeLog, loader.h and loader.c are not consistent. For example, loader.h
declares grub_loader_unregister_preboot_hook, but loader.c defines
grub_loader_remove_preboot.
- I don't understand how preboot_func and preboot_rest_func are different. At
least, not obvious. Can you elaborate on them?
- This part:
+
+ for (cur = preboots_head; cur; cur = cur->next)
+ if (err = cur->preboot_func (grub_loader_noreturn))
+ {
+ for (cur = cur->prev; cur; cur = cur->prev)
+ cur->preboot_rest_func ();
+ return err;
+ }
You should not set ERR inside the "if" clause. This is against the GNU Coding
Standards. The main reason is that it is not friendly to debuggers (as you
may not "step" between the assignment and the conditional jump).
Regards,
Okuji
- [PATCH] Preboot support, phcoder, 2009/04/10
- Re: [PATCH] Preboot support, Yoshinori K. Okuji, 2009/04/11
- Re: [PATCH] Preboot support, phcoder, 2009/04/11
- Re: [PATCH] Preboot support, phcoder, 2009/04/12
- Re: [PATCH] Preboot support,
Yoshinori K. Okuji <=
- Re: [PATCH] Preboot support, phcoder, 2009/04/15
- Re: [PATCH] Preboot support, John Stanley, 2009/04/15
- Re: [PATCH] Preboot support, Vladimir 'phcoder' Serbinenko, 2009/04/26
- Re: [PATCH] Preboot support, Vladimir 'phcoder' Serbinenko, 2009/04/27