Re: [coreboot] Coreboot bug?

[Myles Watson]

On Wed, Nov 11, 2009 at 3:21 PM, Peter Stuge  wrote:
> Stefan Reinauer wrote:
> > > Maybe make a type with struct lb_record followed by unsigned char [],
> > > then it is more clear what is going on.
> >
> > Or just get the length via read32.
>
> I was thinking about how the source code looks, not so much what
> happens at run time. But that could also be an improvement!

How about this:

Index: src/arch/i386/boot/coreboot_table.c
===================================================================
--- src/arch/i386/boot/coreboot_table.c	(revision 4931)
+++ src/arch/i386/boot/coreboot_table.c	(working copy)
@@ -485,11 +485,10 @@

#if (CONFIG_HAVE_OPTION_TABLE == 1)
    {
-		struct lb_record *rec_dest, *rec_src;
-		/* Write the option config table... */
+		struct lb_record *rec_dest;
+		/* Copy the option config table, it's already a lb_record... */
    	rec_dest = lb_new_record(head);
-		rec_src = (struct lb_record *)(void *)&option_table;
-		memcpy(rec_dest,  rec_src, rec_src->size);
+		memcpy(rec_dest,  &option_table, sizeof(option_table));
    	/* Create cmos checksum entry in coreboot table */
    	lb_cmos_checksum(head);
    }

Since it's already formatted correctly, we don't try to do anything
special.  We just copy it into place.  I guess we could put in a check
to make sure the second u32 equals sizeof(option_table), but it seems
like we implicitly trust the tool that created it.

Boot-tested on qemu.

Signed-off-by: Myles Watson

Thanks,
Myles