Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, David Hendricks, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52362
to look at the new patch set (#21).
Change subject: cli_classic.c: Make -r/-w/-v argument optional
......................................................................
cli_classic.c: Make -r/-w/-v argument optional
Makes filename optional from -r/-w/-v since the -i parameter allows
building the image to be written from multiple files, and regions to be
read from flash and written to separate image files,
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
---
M cli_classic.c
M flashrom.c
M layout.c
M layout.h
4 files changed, 95 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/52362/21
--
To view, visit https://review.coreboot.org/c/flashrom/+/52362
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
Gerrit-Change-Number: 52362
Gerrit-PatchSet: 21
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Daniel Campello, Anastasia Klimchuk.
Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/59921
to look at the new patch set (#2).
Change subject: flashrom.c: extract operation only uses layout files
......................................................................
flashrom.c: extract operation only uses layout files
This change fixes a bug on handling the extract operation. The extract
operation reads out the layout regions to filenames corresponding to the
respective layout region names. read_flash_to_file() does this work via
write_buf_to_include_args(). This change makes the call to
write_buf_to_file() optional as it is still required for -r (read
operation) but not for -x (extract operation).
BUG=b:209512852
TEST=flashrom -x
Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
Signed-off-by: Daniel Campello <campello(a)chromium.org>
---
M flashrom.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/59921/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/59921
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
Gerrit-Change-Number: 59921
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Daniel Campello 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:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/59291/comment/69c37d93_e00378a6
PS4, Line 2185: ret = write_buf_to_file(buf, size, filename);
Just add "if (filename)" as in CB:59921
--
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(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 06 Dec 2021 23:51:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Daniel Campello 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: Code-Review+1
--
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(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 06 Dec 2021 23:49:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Daniel Campello, Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59921 )
Change subject: flashrom.c: extract operation only uses layout files
......................................................................
Patch Set 1:
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/59921/comment/2f6a9ab5_221551cb
PS1, Line 9: Extract
: operation reads the layout regions to filenames with equal name to the
: specified layout regions.
Do you mean to say -
"The extract operation reads out the layout regions to filenames corresponding to the respective layout region names."
The previous sentence is sort of hard to parse.
https://review.coreboot.org/c/flashrom/+/59921/comment/8f2f6fbe_f8211fd3
PS1, Line 16: C
upstream needs a sign-off line
https://review.coreboot.org/c/flashrom/+/59921/comment/9ddc1300_755ef069
PS1, Line 16: Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
BUG=/TEST= are useful.
Patchset:
PS1:
Thanks for the patch!
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/59921/comment/5ba3d812_75471ad3
PS1, Line 1076: read_flash_to_file(
as this relates, could you take a look at https://review.coreboot.org/c/flashrom/+/59291 please Daniel?
--
To view, visit https://review.coreboot.org/c/flashrom/+/59921
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
Gerrit-Change-Number: 59921
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 06 Dec 2021 23:10:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Daniel Campello has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/59921 )
Change subject: flashrom.c: extract operation only uses layout files
......................................................................
flashrom.c: extract operation only uses layout files
This change fixes a bug on handling the extract operation. Extract
operation reads the layout regions to filenames with equal name to the
specified layout regions. read_flash_to_file() does this work via
write_buf_to_include_args(). This change makes the call to
write_buf_to_file() optional as it is still required for -r (read
operation) but not for -x (extract operation).
Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
---
M flashrom.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/59921/1
diff --git a/flashrom.c b/flashrom.c
index 48d953b..632bccd 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1100,7 +1100,8 @@
goto out_free;
}
- ret = write_buf_to_file(buf, size, filename);
+ if (filename)
+ ret = write_buf_to_file(buf, size, filename);
out_free:
free(buf);
msg_cinfo("%s.\n", ret ? "FAILED" : "done");
--
To view, visit https://review.coreboot.org/c/flashrom/+/59921
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
Gerrit-Change-Number: 59921
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59532 )
Change subject: [FIX ME] tests: Add test for extract operation
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> But the reference patch already exists (CB:52362), why would I duplicate a chunk of it in another pa […]
I disagree... read_flash_to_file is still writing to files (via write_buf_to_include_args) when filename is NULL. If you read the function it is reading first the contents of flash into a memory buffer and then using that buffer to write to the files specified in the include args (which respect the layout portions) and then (with the proposed modification) optionally writing the whole buffer (with empty bytes where the layout does not have a defined region) to the specified filename.
Just calling write_buf_to_include_args would not make sense without first reading from flash to memory buffer
With respect of CB:52362 the CL introduces a change on the CLI interface, and that can be treated separately than the implementation of the extract method (which has a bug without the "if (filename)" in read_flash_to_file. Adding this should not affect in any way the existing requirement of a filename for the other options (-r/-w/-v) which is enforced at higher levels in the stack regardless.
--
To view, visit https://review.coreboot.org/c/flashrom/+/59532
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I13107c0e095d53184e32a41fa72227cf7dc1d449
Gerrit-Change-Number: 59532
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Mon, 06 Dec 2021 19:03:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Daniel Campello <campello(a)google.com>
Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment