Re: Sendkey patch

[Javier Martín]

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.

preboot.patch
Description: Text Data

signature.asc
Description: Esta parte del mensaje está firmada digitalmente