Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38421 )
Change subject: commonlib/bsd: Add new CBFS core implementation ......................................................................
Patch Set 16:
(7 comments)
https://review.coreboot.org/c/coreboot/+/38421/4/src/commonlib/bsd/cbfs_priv... File src/commonlib/bsd/cbfs_private.c:
https://review.coreboot.org/c/coreboot/+/38421/4/src/commonlib/bsd/cbfs_priv... PS4, Line 54: goto next_file;
I was just trying to continue where possible and skip over broken files rather than abort completely […]
(Continued in https://review.coreboot.org/c/coreboot/+/38421/11/src/commonlib/bsd/cbfs_pri...)
https://review.coreboot.org/c/coreboot/+/38421/4/src/commonlib/bsd/cbfs_priv... PS4, Line 123:
cbfs_walk() checks (data_offset > sizeof(mdata)) and (data_offset < attr_offset), so I think these c […]
(Continued in https://review.coreboot.org/c/coreboot/+/38421/11/src/commonlib/bsd/cbfs_pri...)
https://review.coreboot.org/c/coreboot/+/38421/8/src/commonlib/bsd/cbfs_priv... File src/commonlib/bsd/cbfs_private.c:
https://review.coreboot.org/c/coreboot/+/38421/8/src/commonlib/bsd/cbfs_priv... PS8, Line 8: size_t devsize
Maybe constify this?
Done
https://review.coreboot.org/c/coreboot/+/38421/8/src/commonlib/bsd/cbfs_priv... PS8, Line 30: bool do_hash
Maybe constify this?
Done
https://review.coreboot.org/c/coreboot/+/38421/4/src/commonlib/bsd/include/c... File src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h:
https://review.coreboot.org/c/coreboot/+/38421/4/src/commonlib/bsd/include/c... PS4, Line 69: * the remaining CBFS (but not call |walker| anymore).
This is already supposed to be the underlying workhorse function, the wrappers are things like cbfs_ […]
Resolved?
https://review.coreboot.org/c/coreboot/+/38421/4/src/commonlib/bsd/include/c... PS4, Line 91: offset to the file data
Not sure what you mean, relative to what? This is absolute. […]
Done
https://review.coreboot.org/c/coreboot/+/38421/4/src/commonlib/bsd/include/c... File src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h:
https://review.coreboot.org/c/coreboot/+/38421/4/src/commonlib/bsd/include/c... PS4, Line 131: struct vb2_hash hash;
Yes, this struct is supposed to have a somewhat dynamic length. […]
Done