[coreboot] fix for cbfs error for longer file names

ron minnich rminnich at gmail.com
Tue May 12 17:09:46 CEST 2009


On Tue, May 12, 2009 at 6:05 AM, Myles Watson <mylesgw at gmail.com> wrote:
> On Mon, May 11, 2009 at 10:32 AM, ron minnich <rminnich at gmail.com> wrote:
>> attached.
>>
>> This came up when cbfs (correctly) diagnosed a rom as corrupt ... it
>> was a bug in cbfs, but that's how it's supposed to work -- catch a bad
>> rom at build time, not boot time.
>
> +       csize = headersize(name);
> +
> +       strcpy(c->magic, COMPONENT_MAGIC);
> +
> +       c->offset = htonl(csize);
>
> I think this code would be clearer without csize.
>
> +       c->offset = htonl(headersize(name));

csize is used one other place in that function. I did not change it as
you recommend for that reason.

>
> The only other comment I have is that rom_add is pretty trivial now.
> It could disappear since it is only a wrapper for rom_alloc.  Is there
> any time we want to do rom_alloc when we don't want to copy in the
> data?

I don't know, I had the same question when I changed the code, but did
not want to rule out an "empty allocate", and decided not to change
it.
Note that the special case of create is an empty allocate.

>
> Acked-by: Myles Watson<mylesgw at gmail.com>
>
>> BTW, you can simplify the existing coreboot code a bit if you add a
>> "char filename[0];" entry to 'struct cbfs_file' - as SeaBIOS does.
>
> I like that idea.
>

I do too; I want to get this change in but we ought to schedule this
fix for the next set.

The nice thing is that since cbfstool is decoupled from coreboot and
seabios code, these changes are easy to make.

Committed revision 4276.

The next big step in my view is setting a flash-friendly value of zero.

ron




More information about the coreboot mailing list