Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38421 )
Change subject: commonlib/bsd: Add new CBFS core implementation ......................................................................
Patch Set 12: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/38421/11/src/commonlib/bsd/include/... File src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h:
https://review.coreboot.org/c/coreboot/+/38421/11/src/commonlib/bsd/include/... PS11, Line 4: PRIVATE
It's supposed to be APIs private to the CBFS stack that shouldn't be called directly by any other pa […]
Thanks for the explanation. This sounds good, but please make sure to write a page in documentation to point people at.
https://review.coreboot.org/c/coreboot/+/38421/11/src/commonlib/bsd/include/... PS11, Line 82: enum cbfs_walk_flags
Is it illegal / frowned upon to use an enum type for values that are ORed together from valid enum v […]
I much prefer using an enum over a plain unsigned int. Another option would be to use a struct with custom storage sizes, but I don't know if that would be too cursed.
https://review.coreboot.org/c/coreboot/+/38421/11/src/commonlib/bsd/include/... PS11, Line 94: Verify the metadata with |master_hash| if provided.
Yes, I always use the term "master hash" only for the hash over the metadata of the whole CBFS (the […]
Discarding a const qualifier is a very bad idea, IMHO.