[coreboot] fix for cbfs error for longer file names
Myles Watson
mylesgw at gmail.com
Tue May 12 17:52:50 CEST 2009
> > + 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
More information about the coreboot
mailing list