Attention is currently required from: Anastasia Klimchuk.
Matt DeVillier has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/85159?usp=email )
Change subject: cli_classic.c: Make -r/-w/-v argument optional when using -i ......................................................................
Patch Set 2:
(4 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/85159/comment/c7ef2d96_8070bda9?usp... : PS2, Line 691: if (read_buf_from_include_args(get_layout(flash), newcontents))
Should it be `else if`? We can't have both, right?
I don't think so - we check if there is a filename following -r/-w/-v, and if so read the file into a buffer. Then we check if there is a filename following the region if -i is used, and if so, read that into the same buffer. So the way it works now, the filename following the region takes precedence, which is consistent with existing behavior and the documentation
https://review.coreboot.org/c/flashrom/+/85159/comment/ad117f43_ace3a81c?usp... : PS2, Line 727: if (read_buf_from_include_args(get_layout(flash), newcontents))
Same question, should it be `else if`?
same as above
https://review.coreboot.org/c/flashrom/+/85159/comment/fc14e833_5823a68d?usp... : PS2, Line 1351: then: : * - Do partial read for each -i arg, creating a new file for : * each region where a filename is provided (-i region:filename). : * - Create a ROM-sized file with partially filled content. For each : * -i arg, fill the corresponding offset with content from ROM.
... then filename specified for `-r` is ignored? […]
no, -r is the exception here.
https://review.coreboot.org/c/flashrom/+/85159/comment/35f8d3c8_f2c34dca?usp... : PS2, Line 1357: Rules for writing and verifying:
I know it wasn't you who wrote these Rules initially, but let's make them more readable (it's hard r […]
I went to re-write this section based on your comments, but feel that it is actually correct as-is. I reformatted it slightly for readability.