Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52958 )
Change subject: buspirate_spi.c: Refactor singleton states into reentrant pattern
......................................................................
Patch Set 2:
(2 comments)
File buspirate_spi.c:
https://review.coreboot.org/c/flashrom/+/52958/comment/8328caaa_213fbb26
PS2, Line 54: bp_
The `bp_` prefix seems unnecessary now.
https://review.coreboot.org/c/flashrom/+/52958/comment/522524d6_f0fd9c98
PS2, Line 167: struct buspirate_spi_data *buspirate_data = data;
How about an
unsigned char *const bp_commbuf = buspirate_data.bp_commbuf;
here and similar cases below? that could reduce some noise, both in code and diff.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52958
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8
Gerrit-Change-Number: 52958
Gerrit-PatchSet: 2
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-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: Thu, 06 May 2021 21:59:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Stefan Reinauer, Edward O'Callaghan, Anastasia Klimchuk.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50246 )
Change subject: programmer.h: Convert anon union to anon struct
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/50246
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242
Gerrit-Change-Number: 50246
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Alan Green <avg(a)google.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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 06 May 2021 14:13:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Stefan Reinauer, Anastasia Klimchuk.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50246 )
Change subject: programmer.h: Convert anon union to anon struct
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/50246
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242
Gerrit-Change-Number: 50246
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Alan Green <avg(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 06 May 2021 04:40:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Edward O'Callaghan has submitted this change. ( 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
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52889
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Peter Marheine <pmarheine(a)chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
M util/flashrom_tester/Cargo.toml
M util/flashrom_tester/src/main.rs
2 files changed, 13 insertions(+), 7 deletions(-)
Approvals:
build bot (Jenkins): Verified
Edward O'Callaghan: Looks good to me, approved
Peter Marheine: Looks good to me, but someone else must approve
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: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Daniel Campello, Angel Pons.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52890 )
Change subject: CHROMIUM: flashrom_tester: Remove --ignore-fmap option
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/52890
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib329f3adb59ce1848d1540844d64b968f49eb22a
Gerrit-Change-Number: 52890
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: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 05 May 2021 23:14:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52958
to look at the new patch set (#2).
Change subject: buspirate_spi.c: Refactor singleton states into reentrant pattern
......................................................................
buspirate_spi.c: Refactor singleton states into reentrant pattern
Move global singleton states into a struct and store within
the spi_master data field for the life-time of the driver.
This is one of the steps on the way to move spi_master data
memory management behind the initialisation API, for more
context see other patches under the same topic "register_master_api".
BUG=b:185191942
TEST=builds
Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M buspirate_spi.c
1 file changed, 119 insertions(+), 104 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/58/52958/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/52958
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8
Gerrit-Change-Number: 52958
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset