> > + 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. It looks like a separate (shadowed) declaration.
> > > > 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. I'm not sure we want that, but that's fine. > >> 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. OK > The nice thing is that since cbfstool is decoupled from coreboot and > seabios code, these changes are easy to make. Yes. > Committed revision 4276. > > The next big step in my view is setting a flash-friendly value of zero. Sounds good. Myles -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
