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 8:
(4 comments)
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?
https://review.coreboot.org/c/coreboot/+/38421/8/src/commonlib/bsd/cbfs_priv... PS8, Line 30: bool do_hash Maybe constify this?
https://review.coreboot.org/c/coreboot/+/38421/8/src/commonlib/bsd/cbfs_priv... PS8, Line 44: while ((ret_header = read_next_header(dev, &offset, &mdata.h)) == CB_SUCCESS) {
Who knows what it does... […]
In the case above, fixing it would result in two nested if's, and it would bring the line length past 96 characters. So I wouldn't touch it 😄
I think checkpatch only complains on instances inside if statements. It does not seem to complain on loops, maybe because it does makes sense to use such a construct for loops.
https://review.coreboot.org/c/coreboot/+/38421/8/src/commonlib/bsd/include/c... File src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h:
https://review.coreboot.org/c/coreboot/+/38421/8/src/commonlib/bsd/include/c... PS8, Line 127: /* Actual size in CBFS may be larger/smaller than struct size! */ Silly question: what does this mean?