Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29547 )
Change subject: security/vboot: Add measured boot mode ......................................................................
Patch Set 70:
(3 comments)
I see that maintaining this change-set is cumbersome. But I agree with Julius regarding the coding style issues. There are too long lines introduced.
Werner, can you point to an announcement on the mailing list for the clang-format requirement? It looks to me, that clang-format is still not configured properly to be used by default and blindly. How the clang-format transition should be conducted, should be discussed on the mailing list.
https://review.coreboot.org/#/c/29547/70//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/29547/70//COMMIT_MSG@12 PS70, Line 12: Could you please add on what platforms this was tested?
https://review.coreboot.org/#/c/29547/70/Documentation/security/index.md File Documentation/security/index.md:
PS70: Could be split out.
https://review.coreboot.org/#/c/29547/70/src/security/vboot/vboot_logic.c File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/#/c/29547/70/src/security/vboot/vboot_logic.c@28... PS70, Line 281: return vboot_extend_pcr(ctx, 0, BOOT_MODE_PCR) || vboot_extend_pcr(ctx, 1, HWID_DIGEST_PCR);
line over 80 characters
So clang-format seems to be incorrect here, and should not have touched these lines.
-
Paul Menzel (Code Review)