Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41121 )
Change subject: cbfstool: Support CONFIG_CBFS_VERIFICATION and metadata hash anchor ......................................................................
Patch Set 14:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41121/8/util/cbfstool/cbfstool.c File util/cbfstool/cbfstool.c:
https://review.coreboot.org/c/coreboot/+/41121/8/util/cbfstool/cbfstool.c@91 PS8, Line 91: struct mh_cache {
I don't see comments, if any, in this patch. It's hard to follow intended semantics.
Added a couple of comments.
https://review.coreboot.org/c/coreboot/+/41121/8/util/cbfstool/cbfstool.c@14... PS8, Line 149: (void *)-1
Why are we updating a pointer to this value? -1 is also not a valid pointer size.
I was just using the .region member to check whether the structure has been initialized yet, so I needed to store a non-NULL bogus value here. I instead added an explicit `initialized` struct member now to make it more readable.
https://review.coreboot.org/c/coreboot/+/41121/8/util/cbfstool/cbfstool.c@17... PS8, Line 175: ERROR("Metadata hash anchor no longer where it used to be!\n");
Is that possible? If so, we should discuss how that could happen in the commentary of the code.
No, I don't think it should be. Changed to assert() to clarify that.