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?

-- To view, visit https://review.coreboot.org/c/flashrom/+/59291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4b690b688acf9d5deb46e8642a252a2132ea8c73 Gerrit-Change-Number: 59291 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Daniel Campello <campello@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Daniel Campello <campello@chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm@chromium.org> Gerrit-Comment-Date: Mon, 27 Dec 2021 00:25:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm@chromium.org> Gerrit-MessageType: comment