Attention is currently required from: Peter Marheine, Evan Benn.
Hello build bot (Jenkins), Evan Benn, Peter Marheine,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/flashrom/+/72674
to review the following change.
Change subject: Revert "flashrom_tester: Add positive check to verify_fail_test" ......................................................................
Revert "flashrom_tester: Add positive check to verify_fail_test"
This reverts commit 5735529d6209388079340a6fbd55e222a1c4c1b9.
Reason for revert: Fails to type-check in verify_region_from_file().
Change-Id: Ib8c99de7aca672f42b3159082e2e63e0fefef08f --- M util/flashrom_tester/flashrom/src/cmd.rs M util/flashrom_tester/flashrom/src/flashromlib.rs M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/tests.rs 4 files changed, 15 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/72674/1
diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs index 1f13a8e..85cbb12 100644 --- a/util/flashrom_tester/flashrom/src/cmd.rs +++ b/util/flashrom_tester/flashrom/src/cmd.rs @@ -269,18 +269,6 @@ Ok(()) }
- fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError> { - let opts = FlashromOpt { - io_opt: Some(IOOpt::Verify(OperationArgs::RegionFileRegion( - region, path, None, - ))), - ..Default::default() - }; - - self.dispatch(opts, "verify_region_from_file")?; - Ok(()) - } - fn erase(&self) -> Result<(), FlashromError> { let opts = FlashromOpt { io_opt: Some(IOOpt::Erase), diff --git a/util/flashrom_tester/flashrom/src/flashromlib.rs b/util/flashrom_tester/flashrom/src/flashromlib.rs index 5e1747b..bf09e6d 100644 --- a/util/flashrom_tester/flashrom/src/flashromlib.rs +++ b/util/flashrom_tester/flashrom/src/flashromlib.rs @@ -152,27 +152,6 @@ Ok(()) }
- fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError> { - let mut layout = self.flashrom.borrow_mut().layout_read_fmap_from_rom()?; - layout.include_region(region)?; - let range = layout.get_region_range(region)?; - let region_data = fs::read(path).map_err(|error| error.to_string())?; - if region_data.len() != range.len() { - return Err(format!( - "verify region range ({}) does not match provided file size ({})", - range.len(), - region_data.len() - ) - .into()); - } - let mut buf = vec![0; self.get_size()? as usize]; - buf[range].copy_from_slice(®ion_data); - self.flashrom - .borrow_mut() - .image_verify(&buf, Some(layout))?; - Ok(()) - } - fn erase(&self) -> Result<(), FlashromError> { self.flashrom.borrow_mut().erase()?; Ok(()) diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs index 41393e8..7c86649 100644 --- a/util/flashrom_tester/flashrom/src/lib.rs +++ b/util/flashrom_tester/flashrom/src/lib.rs @@ -133,8 +133,7 @@ /// Read the whole flash to the file specified by `path`. fn read_into_file(&self, path: &Path) -> Result<(), FlashromError>;
- /// Read only a region of the flash into the file specified by `path`. Note - /// the first byte written to the file is the first byte from the region. + /// Read only a region of the flash. fn read_region_into_file(&self, path: &Path, region: &str) -> Result<(), FlashromError>;
/// Write the whole flash to the file specified by `path`. @@ -153,10 +152,6 @@ /// Verify the whole flash against the file specified by `path`. fn verify_from_file(&self, path: &Path) -> Result<(), FlashromError>;
- /// Verify only the region against the file specified by `path`. - /// Note the first byte in the file is matched against the first byte of the region. - fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError>; - /// Erase the whole flash. fn erase(&self) -> Result<(), FlashromError>;
diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index 721a789..847bfec 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -44,7 +44,6 @@ use std::sync::atomic::AtomicBool;
const ELOG_FILE: &str = "/tmp/elog.file"; -const FW_MAIN_B_PATH: &str = "/tmp/FW_MAIN_B.bin";
/// Iterate over tests, yielding only those tests with names matching filter_names. /// @@ -313,16 +312,7 @@
/// Check that flashrom 'verify' will fail if the provided data does not match the chip data. fn verify_fail_test(env: &mut TestEnv) -> TestResult { - env.ensure_golden()?; - // Verify that verify is Ok when the data matches. We verify only a region - // and not the whole chip because coprocessors or firmware may have written - // some data in other regions. - env.cmd - .read_region_into_file(FW_MAIN_B_PATH.as_ref(), "FW_MAIN_B")?; - env.cmd - .verify_region_from_file(FW_MAIN_B_PATH.as_ref(), "FW_MAIN_B")?; - - // Verify that verify is false when the data does not match + // Comparing the flash contents to random data says they're not the same. match env.verify(env.random_data_file()) { Ok(_) => Err("Verification says flash is full of random data".into()), Err(_) => Ok(()),