[coreboot] seemingly wrong code in src/arch/i386/boot/coreboot_table.c

ron minnich rminnich at gmail.com
Sat May 9 06:35:09 CEST 2009


On Fri, May 8, 2009 at 8:35 PM, Myles Watson <mylesgw at gmail.com> wrote:
> On Fri, May 8, 2009 at 9:06 PM, ron minnich <rminnich at gmail.com> wrote:
>> I don't understand this code.
>>
>> Here it is.
>>
>> #if (HAVE_OPTION_TABLE == 1)
>>        {
>>                struct lb_record *rec_dest, *rec_src;
>>                /* Write the option config table... */
>>                rec_dest = lb_new_record(head);
>>                rec_src = (struct lb_record *)(void *)&option_table;
>>                memcpy(rec_dest,  rec_src, rec_src->size);
>>                /* Create cmos checksum entry in coreboot table */
>>                lb_cmos_checksum(head);
>>        }
>> #endif
>>
>>
>> Note the cast of rec_src.
> I guess I'm clueless what that actually does.  It looks like an
> unsigned char* getting cast to a void* then to a lb_record*.
>
> I wouldn't expect that to change the value.

This is an lb_record
struct lb_record {
        uint32_t tag;           /* tag ID */
        uint32_t size;          /* size of record (in bytes) */
};

option_table is an array of bytes, not a struct. I can see if there is
a filled-in option table this might work, but not in this case:
unsigned char option_table[] = {
        };

Which turns into a four byte quantity, which in terms of the lb_record
is a four byte tag of zero and then a length of ... whatever comes
after option_table!
This code needs some fixing. I have a simple hack but I'd rather do
something better than what we have now. It will need repair to the
build_opt_tbl tool.

ron




More information about the coreboot mailing list