Attention is currently required from: Nico Huber, Angel Pons, Daniel Campello, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59291 )
Change subject: flashrom: Convert do_read() into a libflashrom user ......................................................................
Patch Set 4:
(4 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/59291/comment/93a837ee_8eac38e1 PS4, Line 635: if (map_flash(&flashes[0]) != 0) { : free(flashes[0].chip); : ret = 1; : goto out_shutdown; : } : msg_cinfo("Please note that forced reads most likely contain garbage.\n"); : ret = do_read(&flashes[0], filename); : unmap_flash(&flashes[0]);
Mapping/unmapping is redundant with the calls in prepare_/finalize_flash_access(). […]
That is a good point, I removed them. Luckily force should by-pass the safety checks called from prepare_flash_access() WDYT?
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/59291/comment/188ac057_6f6035ab PS4, Line 1935: }
Looks redundant given the checks in prepare_flash_access()?
Ah you are right, verify_range() only happens to have the check twice as it is also called in jedec.c. Done thanks Nico.
https://review.coreboot.org/c/flashrom/+/59291/comment/a1f46f2e_d448c5a3 PS4, Line 2173: msg_cinfo("FAILED.\n");
I'd drop this print.
Done
https://review.coreboot.org/c/flashrom/+/59291/comment/cde92e31_06c47d98 PS4, Line 2189: msg_cinfo("%s.\n", ret ? "FAILED" : "done");
This would print `FAILED` twice if `flashrom_image_read()` took the branch that prints `FAILED`.
I dropped it. Done?