On Wed, Nov 11, 2009 at 3:21 PM, Peter Stuge <[email protected]> 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 <[email protected]>

Thanks,
Myles

-- 
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to