Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37262 )
Change subject: util: cbfstool: Check alignment at build time ......................................................................
Patch Set 6: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/37262/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37262/6//COMMIT_MSG@20 PS6, Line 20: FMAP%@0x1000 0x800 nit: I feel conceptually this would look better as
FMAP@0x1000% 0x800
because alignment is a property of the offset, not the name. (For sections with automatic offset it would still be 'FMAP%', or 'COREBOOT(CBFS)%'.) But that might just be a matter of taste.
https://review.coreboot.org/c/coreboot/+/37262/6/util/cbfstool/fmap_from_fmd... File util/cbfstool/fmap_from_fmd.c:
https://review.coreboot.org/c/coreboot/+/37262/6/util/cbfstool/fmap_from_fmd... PS6, Line 39: Non-CBFS nit: doesn't really have anything to do with CBFS?
https://review.coreboot.org/c/coreboot/+/37262/6/util/cbfstool/fmd_parser.y File util/cbfstool/fmd_parser.y:
https://review.coreboot.org/c/coreboot/+/37262/6/util/cbfstool/fmd_parser.y@... PS6, Line 112: $$.v = $1.v | $3.v; I am unclear why this is $1 and $3 here but $1 and $2 in the seemingly equivalent case above. But I also have no idea how yylex works.