Hans,
could you please address Marco's issues and send a new patch so the
topic is brought up again?
Am Freitag, den 29.08.2008, 18:08 +0200 schrieb Marco Gerards:
>
> > diff -uwr grub-1.96_svn20080813-org/ChangeLog
> > grub-1.96_svn20080813-new/ChangeLog
> > --- grub-1.96_svn20080813-org/ChangeLog 2008-08-13 17:24:36.000000000
> > +0200
> > +++ grub-1.96_svn20080813-new/ChangeLog 2008-08-29 10:33:03.000000000
> > +0200
> > @@ -1,3 +1,8 @@
> > +2008-08-28 Hans Lambermont <[EMAIL PROTECTED]> (tiny change)
> > + Jan Derk Gerlings <[EMAIL PROTECTED]> (tiny change)
> > +
> > + * disk/lvm.c: Add capability to read circular metadata
>
> Please describe changes in the changelog entry, not the effect.
>
> For example:
>
> * disk/lvm.c (grub_lvm_scan_device): Allocate buffer space for the
> worst case scenario.
> (grub_lvm_scan_device): ...
>
> Where ... has to be filled in, I have no idea what this code actually
> does or what you changed ;-)
>
> The tiny change syntax does not seem familiar to me, AFAIK it is not
> from the GCS. Can you please change that? Furthermore, if you both
> worked on there two parts of the patch, please send in separate
> patches. It will make my life a lot easier... :-)
>
> If only one person worked on this, for example Jan Derk, which Hans
> only forwards this patch, please only list Jan Derk as the
> contributor.
>
> > 2008-08-12 Robert Millan <[EMAIL PROTECTED]>
> >
> > * loader/i386/pc/multiboot.c (grub_multiboot_load_elf32): Move part
> > diff -uwr grub-1.96_svn20080813-org/disk/lvm.c
> > grub-1.96_svn20080813-new/disk/lvm.c
> > --- grub-1.96_svn20080813-org/disk/lvm.c 2008-08-28 14:32:53.000000000
> > +0200
> > +++ grub-1.96_svn20080813-new/disk/lvm.c 2008-08-28 18:31:19.000000000
> > +0200
> > @@ -281,7 +281,8 @@
> > goto fail;
> > }
> >
> > - metadatabuf = grub_malloc (mda_size);
> > + /* alloc for circular worst-case scenario */
>
> Nitpick: Please start a sentence with a capital letter and end with a
> `.'. So this will become:
>
> /* Assume circular buffer in a worst case scenario. */
>
>
> > + metadatabuf = grub_malloc (2*mda_size);
>
> Please use spaces around operators:
>
> 2 * md_size
>
> > if (! metadatabuf)
> > goto fail;
> >
> > @@ -300,6 +301,12 @@
> > }
> >
> > rlocn = mdah->raw_locns;
> > + if (rlocn->offset + rlocn->size > mdah->size)
>
> Here rlcon->offset seems to be little endian (64 bits), so please use
> grub_le_to_cpu64. Same for rlocn->size, please check the size of this
> member before you use a macro (I couldn't find it immediately...).
>
> > + {
> > + /* metadata is circular */
>
> Same as above.
>
> > + grub_memcpy(metadatabuf + mda_size, metadatabuf + mdah->start,
> > + ((rlocn->offset + rlocn->size) - mdah->size));
> > + }
>
> Please check and correct the endianess. you use a lot of
> parenthesises. Actually, I think none are requires.
--
Felix Zielcke
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
