Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32532 )
Change subject: security/vboot/vboot_crtm.c: Use ENV_ conditions for vboot_measure_cbfs_hook() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32532/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32532/2//COMMIT_MSG@10 PS2, Line 10: is enabled, but this function is defined as 0 in vboot_crtm.h using ENV_
One or the other way conditions in vboot_crtm.h and vboot_crtm.c should be in sync. […]
vboot_crtm.c is currently guarded by the Kconfig via the Makefile, and nothing else. That's why I'm saying you should take the ENV_ checks out of the header, then they'll be in sync.
Yes, putting an explicit if (...) in the place where it's called would also work and comes out to the same. That's a matter of taste and both approaches are common in coreboot code. (Arguably, the way it's currently done with an empty stub in the header is more common for optional features like this, e.g. timestamps use the same thing. I think the point is to use up less lines in the calling code so that an optional feature doesn't dominate the main code flow that much.)