Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47831 )
Change subject: util/cbfstool: Introduce concept of mmap_window ......................................................................
Patch Set 11:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47831/11/util/cbfstool/cbfstool.c File util/cbfstool/cbfstool.c:
https://review.coreboot.org/c/coreboot/+/47831/11/util/cbfstool/cbfstool.c@1... PS11, Line 131: /* : * pointless nit about comment formatting
https://review.coreboot.org/c/coreboot/+/47831/11/util/cbfstool/cbfstool.c@1... PS11, Line 187: reg = (struct region *)((char *)&mmap_window_table[i] + struct_offset); this is a pretty messy way to get at the structure, any reason you didn't add something like an enum for HOST_SPACE/FLASH_SPACE to pass around instead of using offsetof? I'm just complaining because it takes more work to understand what it is doing when reading the code..
https://review.coreboot.org/c/coreboot/+/47831/11/util/cbfstool/cbfstool.c@2... PS11, Line 219: added might be good to note why this is done before the convert instead of after (like the host_to_flash case). (it makes sense once you think about it but for those attempting to read the code for the first time...)