Attention is currently required from: Sam McNally, Nico Huber, Nicola Corna, Paul Menzel, Stefan Reinauer, David Hendricks, Arthur Heymans, Carl-Daniel Hailfinger. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23021 )
Change subject: layout: Add -i <region>[:<file>] support ......................................................................
Patch Set 27:
(7 comments)
Patchset:
PS27:
Thanks for taking this over. I have to say, however, that it was […]
In the follow up CB:52362 I implement the chromiumos syntax in https://flashrom.org/Per_region_file_arguments I have been working on converge chromium and upstream flashrom and this change plus the two followups would achieve that in regards of syntax. I realize now that CB:23022 was attempting the same 😊
Convergence has advanced quite a bit (crrev.com/c/2823343) that I think striving for the same CLI syntax is the right approach. Also, all of these changes are backward compatible with existent interface.
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/23021/comment/be786e23_6f12a404 PS27, Line 319: tempstr = strdup(optarg);
If register_include_arg() is not going to own it, we should pass it `const` […]
strdup() here since strtok() modifies the string:
layout.c: In function 'register_include_arg': layout.c:123:16: error: passing argument 1 of 'strtok' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] 123 | name = strtok(include, ":");
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/23021/comment/9e166f18_9b5fcc93 PS16, Line 1367: when writing
Any other case when this is used? or, why remove this?
I have been working with the intention of reduce as much as possible the differences between upstream and chromiumos flashrom. This is one case where the string was like that, but I'll add it back and send a patch in chromium.
File layout.c:
https://review.coreboot.org/c/flashrom/+/23021/comment/a0165551_beef69c6 PS21, Line 253: goto out;
The intention obviously was to report all overlapping regions. […]
I overlooked this. Will revert to have the reporting as I don't expect the loop over all regions to be expensive...
File layout.c:
https://review.coreboot.org/c/flashrom/+/23021/comment/f578513c_8177e092 PS27, Line 147: tmp->file = strdup(file);
else?
Done
https://review.coreboot.org/c/flashrom/+/23021/comment/ec603e74_bd313e10 PS27, Line 155: include_region
Could be exported as part of the libflashrom API, e.g. […]
Agree! This could come in a follow up CL!
https://review.coreboot.org/c/flashrom/+/23021/comment/906e2106_3a649916 PS27, Line 221: }
Looks like somewhere in the rebasing reporting of the file names got lost.
Done