Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52450 )
Change subject: cli_classic.c: add -x option for do_extract()
......................................................................
Patch Set 12:
(1 comment)
Patchset:
PS12:
> Daniel, I just realised after hitting submit this is missing the manpage update. […]
CB:52892
--
To view, visit https://review.coreboot.org/c/flashrom/+/52450
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff
Gerrit-Change-Number: 52450
Gerrit-PatchSet: 12
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 04 May 2021 17:46:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Daniel Campello has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52892 )
Change subject: flashrom.8.tmpl: Add man entry for --extract
......................................................................
flashrom.8.tmpl: Add man entry for --extract
This is a follow up change of CB:52450
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: Icc068f5545b6f30ac390b7b815a31e2d61bf4789
---
M flashrom.8.tmpl
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/52892/1
diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
index 4634539..4ae7e71 100644
--- a/flashrom.8.tmpl
+++ b/flashrom.8.tmpl
@@ -47,7 +47,7 @@
.B flashrom \fR[\fB\-h\fR|\fB\-R\fR|\fB\-L\fR|\fB\-z\fR|
\fB\-p\fR <programmername>[:<parameters>] [\fB\-c\fR <chipname>]
(\fB\-\-flash\-name\fR|\fB\-\-flash\-size\fR|
- [\fB\-E\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>]
+ [\fB\-E\fR|\fB\-x\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>]
[(\fB\-l\fR <file>|\fB\-\-ifd|\fB \-\-fmap\fR|\fB\-\-fmap-file\fR <file>)
[\fB\-i\fR <image>[:<file>]]]
[\fB\-n\fR] [\fB\-N\fR] [\fB\-f\fR])]
@@ -136,6 +136,10 @@
.B "\-E, \-\-erase"
Erase the flash ROM chip.
.TP
+.B "\-x, \-\-extract"
+Extract every region defined on the layout from flash ROM chip to a
+file with the same name as the extracted region.
+.TP
.B "\-V, \-\-verbose"
More verbose output. This option can be supplied multiple times
(max. 3 times, i.e.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52892
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Icc068f5545b6f30ac390b7b815a31e2d61bf4789
Gerrit-Change-Number: 52892
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: newchange
Daniel Campello has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52889 )
Change subject: CHROMIUM: flashrom_tester: Drop nix dependency
......................................................................
CHROMIUM: flashrom_tester: Drop nix dependency
We can just use the libc functions directly. This is exactly what nix
does anyway.
BUG=none
TEST=unit tests
BRANCH=none
Original-Change-Id: I45c02f0c71d164bd8f504fe2b8d3acd54e0d5704
Original-Signed-off-by: Chirantan Ekbote <chirantan(a)chromium.org>
Original-Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/…
Original-Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
Original-Reviewed-by: Allen Webb <allenwebb(a)google.com>
Original-Commit-Queue: Allen Webb <allenwebb(a)google.com>
(cherry picked from commit 1ba7dbe83e01d270b6d8d597a079ea3bfeb2117e)
Change-Id: Iea61c65efb04da9cd0bc0bd85a34fc10912ea87b
---
M util/flashrom_tester/Cargo.toml
M util/flashrom_tester/src/main.rs
2 files changed, 13 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/89/52889/1
diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml
index 0898d3c..e7a5820 100644
--- a/util/flashrom_tester/Cargo.toml
+++ b/util/flashrom_tester/Cargo.toml
@@ -18,8 +18,8 @@
chrono = { version = "0.4", optional = true }
clap = { version = "2.33", default-features = false, optional = true }
flashrom = { path = "flashrom/" }
+libc = "0.2"
log = { version = "0.4", features = ["std"] }
-nix = "0.14.1"
rand = "0.6.4"
serde_json = "1"
sys-info = "0.5.7"
diff --git a/util/flashrom_tester/src/main.rs b/util/flashrom_tester/src/main.rs
index e589ee1..80484d2 100644
--- a/util/flashrom_tester/src/main.rs
+++ b/util/flashrom_tester/src/main.rs
@@ -152,12 +152,11 @@
/// Once a signal is trapped, the default behavior is restored (terminating
/// the process) for future signals.
fn handle_sigint() -> &'static AtomicBool {
- use nix::libc::c_int;
- use nix::sys::signal::{self, SigHandler, Signal};
+ use libc::c_int;
use std::sync::atomic::Ordering;
unsafe {
- let _ = signal::signal(Signal::SIGINT, SigHandler::Handler(sigint_handler));
+ let _ = libc::signal(libc::SIGINT, sigint_handler as libc::sighandler_t);
}
static TERMINATE_FLAG: AtomicBool = AtomicBool::new(false);
@@ -169,10 +168,17 @@
test, or press ^C again to exit immediately (possibly bricking your machine).
";
- // Use raw write() because signal-safety is a very hard problem
- let _ = nix::unistd::write(STDERR_FILENO, MESSAGE);
+ // Use raw write() because signal-safety is a very hard problem. Safe because this doesn't
+ // modify any memory.
+ let _ = unsafe {
+ libc::write(
+ STDERR_FILENO,
+ MESSAGE.as_ptr() as *const libc::c_void,
+ MESSAGE.len() as libc::size_t,
+ )
+ };
unsafe {
- let _ = signal::signal(Signal::SIGINT, SigHandler::SigDfl);
+ let _ = libc::signal(libc::SIGINT, libc::SIG_DFL);
}
TERMINATE_FLAG.store(true, Ordering::Release);
}
--
To view, visit https://review.coreboot.org/c/flashrom/+/52889
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iea61c65efb04da9cd0bc0bd85a34fc10912ea87b
Gerrit-Change-Number: 52889
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Simon Glass has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 )
Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
......................................................................
Patch Set 10:
(1 comment)
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/51487/comment/ebec52fb_31e6d7f2
PS10, Line 53: MEC1308_SIO_PORT1
> > Yes, that's right... […]
OK. From my POV I don't see much need to test that the enums/#defines in a header file correspond with values in a test...but I can see situations where it might be useful, such as when testing black-box code.
--
To view, visit https://review.coreboot.org/c/flashrom/+/51487
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc
Gerrit-Change-Number: 51487
Gerrit-PatchSet: 10
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Simon Glass <sjg(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
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: Tue, 04 May 2021 16:42:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Simon Glass <sjg(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Simon Glass.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 )
Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
......................................................................
Patch Set 10:
(1 comment)
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/51487/comment/fefaa940_9eceecb0
PS10, Line 53: MEC1308_SIO_PORT1
> Yes, that's right...but are we trying to check that the header file has the right constant values? To me a more useful test is that they 'come through' or are dealt with as expected.
I don't know. FWIW, we have not specified what should be tested to
what extend. With wrong numbers, it wouldn't work, I suppose, so
seems worth to test them too?
>
> If the definitions are considered private, they could go in a header file in the same directory as the driver, perhaps with a _private suffix? Then, just the test can include them
Yeah, that would avoid any confusion with an API.
--
To view, visit https://review.coreboot.org/c/flashrom/+/51487
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc
Gerrit-Change-Number: 51487
Gerrit-PatchSet: 10
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Simon Glass <sjg(a)chromium.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-Attention: Simon Glass <sjg(a)chromium.org>
Gerrit-Comment-Date: Tue, 04 May 2021 16:39:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Simon Glass <sjg(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Simon Glass has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 )
Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
......................................................................
Patch Set 10:
(1 comment)
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/51487/comment/44701e00_7dd46aae
PS10, Line 53: MEC1308_SIO_PORT1
> It depends much on what one wants to test. Sharing the numbers with the […]
Yes, that's right...but are we trying to check that the header file has the right constant values? To me a more useful test is that they 'come through' or are dealt with as expected.
If the definitions are considered private, they could go in a header file in the same directory as the driver, perhaps with a _private suffix? Then, just the test can include them
--
To view, visit https://review.coreboot.org/c/flashrom/+/51487
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc
Gerrit-Change-Number: 51487
Gerrit-PatchSet: 10
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Simon Glass <sjg(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
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: Tue, 04 May 2021 16:15:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Simon Glass <sjg(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Simon Glass.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 )
Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
......................................................................
Patch Set 10:
(1 comment)
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/51487/comment/7845e73b_4e58eb98
PS10, Line 53: MEC1308_SIO_PORT1
> Just to add one comment, adding tests to code that has not been written with testing in mind is ofte […]
It depends much on what one wants to test. Sharing the numbers with the
tests implies _not_ testing if flashrom (still) uses the correct numbers.
I don't know much about designing code for testing. Pulling such
definitions out of their compilation unit makes it easier to write
tests I suppose. But it also makes it harder to read/review the code.
--
To view, visit https://review.coreboot.org/c/flashrom/+/51487
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc
Gerrit-Change-Number: 51487
Gerrit-PatchSet: 10
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Simon Glass <sjg(a)chromium.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-Attention: Simon Glass <sjg(a)chromium.org>
Gerrit-Comment-Date: Tue, 04 May 2021 16:06:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Simon Glass <sjg(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 6
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Richard Hughes <hughsient(a)gmail.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 04 May 2021 15:52:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52876 )
Change subject: flashrom_tester: update syntax for --wp-range
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/52876/comment/a03cfab9_39a64984
PS1, Line 12:
> Nit: Please remove the blank line.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/52876
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If17a40fba1f7d41e09e0163b353d1602c215c8db
Gerrit-Change-Number: 52876
Gerrit-PatchSet: 2
Gerrit-Owner: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 04 May 2021 15:50:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment