Re: Sendkey patch

["Javier Martín" <lordhabbit@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1269 bytes --]

El mar, 02-09-2008 a las 20:39 +0200, phcoder escribió:
> +void
> +grub_loader_remove_preboot (void *p)
> +{
> +  if (!p)
> +    return;
> +  *(PREBOOT_HND(p)->prev_pointer)=PREBOOT_HND(p)->next;
This line will "crash" if p is the head of the list (with prev_pointer
being 0). I quote crash because a crash is what happens under an OS:
under GRUB you just overwrite address 0x0 which in i386-pc is the start
of the real mode IVT.

> +  if (PREBOOT_HND(p)->next)
> +    PREBOOT_HND(p)->next->prev_pointer=PREBOOT_HND(p)->prev_pointer;
> +    grub_free (p);
> +}
All these macro plays are nonsense and a hindrance to readability just
because you did not want to add a local variable and do the cast once.

Here is my "version" of your patch, without the double indirection and
the strange plays. The overhead is 103 bytes without the error line
against 63 of yours, but I really think that the symmetric and
understandable handling of previous and next is worth 40 bytes.

PS:
> +void *EXPORT_FUNC(grub_loader_add_preboot) (grub_err_t ...
I think that (only) in function declarations it's better to write "void*
f()" than "void *f()" because otherwise the * can be easily overlooked.
However, this is my word and does not come from the GCS.

[-- Attachment #1.2: preboot.patch --]
[-- Type: text/x-patch, Size: 2862 bytes --]

Index: kern/loader.c
===================================================================
--- kern/loader.c	(revisión: 1845)
+++ kern/loader.c	(copia de trabajo)
@@ -22,12 +22,54 @@
 #include <grub/err.h>
 #include <grub/kernel.h>
 
+struct grub_preboot_t
+{
+  grub_err_t (*preboot_func) (int);  
+  struct grub_preboot_t *next;
+  struct grub_preboot_t *previous;
+};
+
 static grub_err_t (*grub_loader_boot_func) (void);
 static grub_err_t (*grub_loader_unload_func) (void);
 static int grub_loader_noreturn;
 
 static int grub_loader_loaded;
 
+static struct grub_preboot_t *grub_loader_preboots=0;
+
+void *
+grub_loader_add_preboot (grub_err_t (*preboot_func) (int))
+{
+  struct grub_preboot_t *cur;
+  if (!preboot_func)
+    return 0;
+  cur = (struct grub_preboot_t *) grub_malloc (sizeof (struct grub_preboot_t));
+  if (!cur)
+    {
+      grub_error (GRUB_ERR_OUT_OF_MEMORY, "no memory left to register hook");
+      return 0;
+    }
+  cur->next = grub_loader_preboots;
+  cur->previous = 0;
+  cur->next->previous = cur;
+  cur->preboot_func = preboot_func;
+  grub_loader_preboots = cur;
+  return cur;
+}
+
+void
+grub_loader_remove_preboot (void *p)
+{
+  struct grub_preboot_t *hnd = (struct grub_preboot_t *)p;
+  if (!hnd)
+    return;
+  if (hnd->previous)
+    hnd->previous->next = hnd->next;
+  if (hnd->next)
+    hnd->next->previous = hnd->previous;
+  grub_free (hnd);
+}
+
 int
 grub_loader_is_loaded (void)
 {
@@ -64,12 +106,18 @@
 grub_err_t
 grub_loader_boot (void)
 {
+  struct grub_preboot_t *iter=grub_loader_preboots;
   if (! grub_loader_loaded)
     return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
 
   if (grub_loader_noreturn)
     grub_machine_fini ();
-  
+  while (iter)
+    {
+      if (iter->preboot_func)
+	iter->preboot_func (grub_loader_noreturn);
+      iter=iter->next;
+    }
   return (grub_loader_boot_func) ();
 }
 
Index: include/grub/loader.h
===================================================================
--- include/grub/loader.h	(revisión: 1845)
+++ include/grub/loader.h	(copia de trabajo)
@@ -25,6 +25,8 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+typedef grub_err_t (*grub_preboot_func) (int noreturn);
+
 /* Check if a loader is loaded.  */
 int EXPORT_FUNC(grub_loader_is_loaded) (void);
 
@@ -37,6 +39,12 @@
 /* Unset current loader, if any.  */
 void EXPORT_FUNC(grub_loader_unset) (void);
 
+/*Add a preboot function*/
+void* EXPORT_FUNC(grub_loader_add_preboot) (grub_preboot_func func);
+
+/*Remove given preboot function*/
+void EXPORT_FUNC(grub_loader_remove_preboot) (void *hnd);
+
 /* Call the boot hook in current loader. This may or may not return,
    depending on the setting by grub_loader_set.  */
 grub_err_t EXPORT_FUNC(grub_loader_boot) (void);

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]