Re: [PATCH] decouple mmap parsing and implement Multiboot mmap in the loader

[Marco Gerards <mgerards@xs4all.nl>]

Robert Millan <rmh@aybabtu.com> writes:

> On Wed, Aug 13, 2008 at 07:52:59PM +0200, Marco Gerards wrote:
>> >  #include <grub/types.h>
>> >  #include <grub/symbol.h>
>> > +#include <grub/multiboot.h>	/* For struct grub_mmap_entry, which is also
>> > +				   needed by Multiboot.  */
>> 
>> Isn't it better to split the header file?  This seems like a hack.
>
> The definition is part of the Multiboot spec, so it really needs to be present
> in multiboot.h.
>
> We could also define it separately as grub_mmap_entry in multiboot.h and keep
> the grub_machine_mmap_entry definition in pc/init.h.  Then other arches could
> have their own grub_machine_mmap_entry variant which _does_ differ from
> grub_mmap_entry (like coreboot).
>
> What do you think?

This sounds fine to me.
   
>> > Index: loader/i386/pc/multiboot.c
>> > ===================================================================
>> > --- loader/i386/pc/multiboot.c	(revision 1802)
>> > +++ loader/i386/pc/multiboot.c	(working copy)
>> > @@ -78,14 +78,60 @@
>> >        grub_free ((void *) mbi->cmdline);
>> >        grub_free (mbi);
>> >      }
>> > -
>> > -
>> > +  
>> 
>> Hm? :-)
>
> Some minor janitor work ;-)

:-)

--
Marco