On Tue, May 12, 2009 at 6:05 AM, Myles Watson mylesgw@gmail.com wrote:
On Mon, May 11, 2009 at 10:32 AM, ron minnich rminnich@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 Watsonmylesgw@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