[coreboot] Coreboot bug?
Myles Watson
mylesgw at gmail.com
Wed Nov 11 23:39:49 CET 2009
On Wed, Nov 11, 2009 at 3:21 PM, Peter Stuge <peter at stuge.se> 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 <mylesgw at gmail.com>
Thanks,
Myles
More information about the coreboot
mailing list