Nico Huber has posted comments on this change. ( https://review.coreboot.org/23203 )
Change subject: Add support to get layout from fmap (e.g. coreboot rom) ......................................................................
Patch Set 22:
(5 comments)
Looks good, just some minor comments. I have only looked at the diff of different patch sets lately, will try to have a final look over everything soon.
https://review.coreboot.org/#/c/23203/20/cli_classic.c File cli_classic.c:
https://review.coreboot.org/#/c/23203/20/cli_classic.c@95 PS20, Line 95: int main(int argc, char *argv[])
I just remembered that we have a read_buf_from_file() function in flashrom.c.
Well, you could have kept read_file() and called read_buf_from_file() from there. Doesn't matter.
Would be nice to have a file handling library eventually to make this stuff more obvious.
Agreed.
https://review.coreboot.org/#/c/23203/22/cli_classic.c File cli_classic.c:
https://review.coreboot.org/#/c/23203/22/cli_classic.c@619 PS22, Line 619: out out_shutdown
https://review.coreboot.org/#/c/23203/22/flashrom.8.tmpl File flashrom.8.tmpl:
https://review.coreboot.org/#/c/23203/22/flashrom.8.tmpl@219 PS22, Line 219: (e.g. a coreboot rom image) in the argument and use it to create a layout. leftover
https://review.coreboot.org/#/c/23203/20/fmap.h File fmap.h:
https://review.coreboot.org/#/c/23203/20/fmap.h@70 PS20, Line 70: I should have mentioned, that I didn't mark all occurrences ;)
https://review.coreboot.org/#/c/23203/22/fmap.c File fmap.c:
https://review.coreboot.org/#/c/23203/22/fmap.c@153 PS22, Line 153: if (!flashctx || !flashctx->chip) : return 1; Redundant, probably leftover?