[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 

- 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).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]