Evan Benn has uploaded this change for review.
flashrom_tester: Use Path type and tempdir for test data
Use tempdir crate to create a tempdir that is deleted when the tester
closes. Update many str to Path.
BUG=b:243460685
BRANCH=None
TEST=/usr/bin/flashrom_tester --flashrom_binary /usr/sbin/flashrom host
Change-Id: I4a0b25c8c70fa6dfd682fba3db7d3bf06b2eb7ac
Signed-off-by: Evan Benn <evanbenn@chromium.org>
---
M util/flashrom_tester/Cargo.toml
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/rand_util.rs
M util/flashrom_tester/src/tester.rs
M util/flashrom_tester/src/tests.rs
7 files changed, 140 insertions(+), 94 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/68643/1
diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml
index b57f04e..364a223 100644
--- a/util/flashrom_tester/Cargo.toml
+++ b/util/flashrom_tester/Cargo.toml
@@ -23,6 +23,7 @@
rand = "0.6.4"
serde_json = "1"
sys-info = "0.9"
+tempdir = "0.3.7"
[build-dependencies]
built = { version = "0.5", features = ["chrono"] }
diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs
index faf597d..4078910 100644
--- a/util/flashrom_tester/flashrom/src/cmd.rs
+++ b/util/flashrom_tester/flashrom/src/cmd.rs
@@ -35,15 +35,19 @@
use crate::{FlashChip, FlashromError, ROMWriteSpecifics};
-use std::process::Command;
+use std::{
+ ffi::{OsStr, OsString},
+ path::Path,
+ process::Command,
+};
#[derive(Default)]
pub struct FlashromOpt<'a> {
pub wp_opt: WPOpt,
pub io_opt: IOOpt<'a>,
- pub layout: Option<&'a str>, // -l <file>
- pub image: Option<&'a str>, // -i <name>
+ pub layout: Option<&'a Path>, // -l <file>
+ pub image: Option<&'a str>, // -i <name>
pub flash_name: bool, // --flash-name
pub verbose: bool, // -V
@@ -60,11 +64,11 @@
#[derive(Default)]
pub struct IOOpt<'a> {
- pub read: Option<&'a str>, // -r <file>
- pub write: Option<&'a str>, // -w <file>
- pub verify: Option<&'a str>, // -v <file>
- pub erase: bool, // -E
- pub region: Option<(&'a str, &'a str)>, // --image
+ pub read: Option<&'a Path>, // -r <file>
+ pub write: Option<&'a Path>, // -w <file>
+ pub verify: Option<&'a Path>, // -v <file>
+ pub erase: bool, // -E
+ pub region: Option<(&'a str, &'a Path)>, // --image
}
#[derive(PartialEq, Eq, Debug)]
@@ -227,7 +231,7 @@
}
}
- fn read_into_file(&self, path: &str) -> Result<(), FlashromError> {
+ fn read_into_file(&self, path: &Path) -> Result<(), FlashromError> {
let opts = FlashromOpt {
io_opt: IOOpt {
read: Some(path),
@@ -240,7 +244,7 @@
Ok(())
}
- fn read_region_into_file(&self, path: &str, region: &str) -> Result<(), FlashromError> {
+ fn read_region_into_file(&self, path: &Path, region: &str) -> Result<(), FlashromError> {
let opts = FlashromOpt {
io_opt: IOOpt {
region: Some((region, path)),
@@ -253,7 +257,7 @@
Ok(())
}
- fn write_from_file(&self, path: &str) -> Result<(), FlashromError> {
+ fn write_from_file(&self, path: &Path) -> Result<(), FlashromError> {
let opts = FlashromOpt {
io_opt: IOOpt {
write: Some(path),
@@ -266,7 +270,7 @@
Ok(())
}
- fn verify_from_file(&self, path: &str) -> Result<(), FlashromError> {
+ fn verify_from_file(&self, path: &Path) -> Result<(), FlashromError> {
let opts = FlashromOpt {
io_opt: IOOpt {
verify: Some(path),
@@ -297,8 +301,8 @@
}
}
-fn flashrom_decode_opts(opts: FlashromOpt) -> Vec<String> {
- let mut params = Vec::<String>::new();
+fn flashrom_decode_opts(opts: FlashromOpt) -> Vec<OsString> {
+ let mut params = Vec::<OsString>::new();
// ------------ WARNING !!! ------------
// each param must NOT contain spaces!
@@ -307,58 +311,62 @@
// wp_opt
if opts.wp_opt.range.is_some() {
let (x0, x1) = opts.wp_opt.range.unwrap();
- params.push("--wp-range".to_string());
- params.push(hex_range_string(x0, x1));
+ params.push("--wp-range".into());
+ params.push(hex_range_string(x0, x1).into());
}
if opts.wp_opt.status {
- params.push("--wp-status".to_string());
+ params.push("--wp-status".into());
} else if opts.wp_opt.list {
- params.push("--wp-list".to_string());
+ params.push("--wp-list".into());
} else if opts.wp_opt.enable {
- params.push("--wp-enable".to_string());
+ params.push("--wp-enable".into());
} else if opts.wp_opt.disable {
- params.push("--wp-disable".to_string());
+ params.push("--wp-disable".into());
}
// io_opt
if let Some((region, path)) = opts.io_opt.region {
- params.push("--image".to_string());
- params.push(format!("{}:{}", region, path));
- params.push("-r".to_string());
+ params.push("--image".into());
+ let mut p = OsString::new();
+ p.push(region);
+ p.push(":");
+ p.push(path);
+ params.push(p);
+ params.push("-r".into());
} else if opts.io_opt.read.is_some() {
- params.push("-r".to_string());
- params.push(opts.io_opt.read.unwrap().to_string());
+ params.push("-r".into());
+ params.push(opts.io_opt.read.unwrap().into());
} else if opts.io_opt.write.is_some() {
- params.push("-w".to_string());
- params.push(opts.io_opt.write.unwrap().to_string());
+ params.push("-w".into());
+ params.push(opts.io_opt.write.unwrap().into());
} else if opts.io_opt.verify.is_some() {
- params.push("-v".to_string());
- params.push(opts.io_opt.verify.unwrap().to_string());
+ params.push("-v".into());
+ params.push(opts.io_opt.verify.unwrap().into());
} else if opts.io_opt.erase {
- params.push("-E".to_string());
+ params.push("-E".into());
}
// misc_opt
if opts.layout.is_some() {
- params.push("-l".to_string());
- params.push(opts.layout.unwrap().to_string());
+ params.push("-l".into());
+ params.push(opts.layout.unwrap().into());
}
if opts.image.is_some() {
- params.push("-i".to_string());
- params.push(opts.image.unwrap().to_string());
+ params.push("-i".into());
+ params.push(opts.image.unwrap().into());
}
if opts.flash_name {
- params.push("--flash-name".to_string());
+ params.push("--flash-name".into());
}
if opts.verbose {
- params.push("-V".to_string());
+ params.push("-V".into());
}
params
}
-fn flashrom_dispatch<S: AsRef<str>>(
+fn flashrom_dispatch<S: AsRef<OsStr>>(
path: &str,
params: &[S],
fc: FlashChip,
@@ -366,7 +374,7 @@
) -> Result<(String, String), FlashromError> {
// from man page:
// ' -p, --programmer <name>[:parameter[,parameter[,parameter]]] '
- let mut args: Vec<&str> = vec!["-p", FlashChip::to(fc)];
+ let mut args: Vec<&OsStr> = vec![OsStr::new("-p"), OsStr::new(FlashChip::to(fc))];
args.extend(params.iter().map(S::as_ref));
info!("flashrom_dispatch() running: {} {:?}", path, args);
@@ -454,6 +462,8 @@
#[cfg(test)]
mod tests {
+ use std::path::Path;
+
use super::flashrom_decode_opts;
use super::{FlashromOpt, IOOpt, WPOpt};
@@ -515,21 +525,21 @@
test_io_opt(
IOOpt {
- read: Some("foo.bin"),
+ read: Some(Path::new("foo.bin")),
..Default::default()
},
&["-r", "foo.bin"],
);
test_io_opt(
IOOpt {
- write: Some("bar.bin"),
+ write: Some(Path::new("bar.bin")),
..Default::default()
},
&["-w", "bar.bin"],
);
test_io_opt(
IOOpt {
- verify: Some("/tmp/baz.bin"),
+ verify: Some(Path::new("/tmp/baz.bin")),
..Default::default()
},
&["-v", "/tmp/baz.bin"],
@@ -548,7 +558,7 @@
//use Default::default;
assert_eq!(
flashrom_decode_opts(FlashromOpt {
- layout: Some("TestLayout"),
+ layout: Some(Path::new("TestLayout")),
..Default::default()
}),
&["-l", "TestLayout"]
diff --git a/util/flashrom_tester/flashrom/src/flashromlib.rs b/util/flashrom_tester/flashrom/src/flashromlib.rs
index d20dbb2..3036394 100644
--- a/util/flashrom_tester/flashrom/src/flashromlib.rs
+++ b/util/flashrom_tester/flashrom/src/flashromlib.rs
@@ -34,7 +34,7 @@
use libflashrom::{Chip, Programmer};
-use std::{cell::RefCell, convert::TryFrom, fs};
+use std::{cell::RefCell, convert::TryFrom, fs, path::Path};
use crate::{FlashChip, FlashromError, ROMWriteSpecifics};
@@ -108,13 +108,13 @@
self.wp_range((0, self.get_size()?), en)
}
- fn read_into_file(&self, path: &str) -> Result<(), FlashromError> {
+ fn read_into_file(&self, path: &Path) -> Result<(), FlashromError> {
let buf = self.flashrom.borrow_mut().image_read(None)?;
fs::write(path, buf).map_err(|error| error.to_string())?;
Ok(())
}
- fn read_region_into_file(&self, path: &str, region: &str) -> Result<(), FlashromError> {
+ fn read_region_into_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)?;
@@ -123,7 +123,7 @@
Ok(())
}
- fn write_from_file(&self, path: &str) -> Result<(), FlashromError> {
+ fn write_from_file(&self, path: &Path) -> Result<(), FlashromError> {
let mut buf = fs::read(path).map_err(|error| error.to_string())?;
self.flashrom.borrow_mut().image_write(&mut buf, None)?;
Ok(())
@@ -143,7 +143,7 @@
Ok(true)
}
- fn verify_from_file(&self, path: &str) -> Result<(), FlashromError> {
+ fn verify_from_file(&self, path: &Path) -> Result<(), FlashromError> {
let buf = fs::read(path).map_err(|error| error.to_string())?;
self.flashrom.borrow_mut().image_verify(&buf, None)?;
Ok(())
diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs
index 7e245ae..11874f9 100644
--- a/util/flashrom_tester/flashrom/src/lib.rs
+++ b/util/flashrom_tester/flashrom/src/lib.rs
@@ -39,7 +39,7 @@
mod cmd;
mod flashromlib;
-use std::{error, fmt};
+use std::{error, fmt, path::Path};
pub use cmd::{dut_ctrl_toggle_wp, FlashromCmd};
pub use flashromlib::FlashromLib;
@@ -118,8 +118,8 @@
}
pub struct ROMWriteSpecifics<'a> {
- pub layout_file: Option<&'a str>,
- pub write_file: Option<&'a str>,
+ pub layout_file: Option<&'a Path>,
+ pub write_file: Option<&'a Path>,
pub name_file: Option<&'a str>,
}
@@ -146,16 +146,16 @@
fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError>;
/// Read the whole flash to the file specified by `path`.
- fn read_into_file(&self, path: &str) -> Result<(), FlashromError>;
+ fn read_into_file(&self, path: &Path) -> Result<(), FlashromError>;
/// Read only a region of the flash.
- fn read_region_into_file(&self, path: &str, region: &str) -> Result<(), FlashromError>;
+ fn read_region_into_file(&self, path: &Path, region: &str) -> Result<(), FlashromError>;
/// Write the whole flash to the file specified by `path`.
- fn write_from_file(&self, path: &str) -> Result<(), FlashromError>;
+ fn write_from_file(&self, path: &Path) -> Result<(), FlashromError>;
/// Verify the whole flash against the file specified by `path`.
- fn verify_from_file(&self, path: &str) -> Result<(), FlashromError>;
+ fn verify_from_file(&self, path: &Path) -> Result<(), FlashromError>;
/// Erase the whole flash.
fn erase(&self) -> Result<(), FlashromError>;
diff --git a/util/flashrom_tester/src/rand_util.rs b/util/flashrom_tester/src/rand_util.rs
index 1b0912b..a040c89 100644
--- a/util/flashrom_tester/src/rand_util.rs
+++ b/util/flashrom_tester/src/rand_util.rs
@@ -36,10 +36,11 @@
use std::fs::File;
use std::io::prelude::*;
use std::io::BufWriter;
+use std::path::Path;
use rand::prelude::*;
-pub fn gen_rand_testdata(path: &str, size: usize) -> std::io::Result<()> {
+pub fn gen_rand_testdata(path: &Path, size: usize) -> std::io::Result<()> {
let mut buf = BufWriter::new(File::create(path)?);
let mut a: Vec<u8> = vec![0; size];
@@ -58,8 +59,8 @@
fn gen_rand_testdata() {
use super::gen_rand_testdata;
- let path0 = "/tmp/idk_test00";
- let path1 = "/tmp/idk_test01";
+ let path0 = Path::new("/tmp/idk_test00");
+ let path1 = Path::new("/tmp/idk_test01");
let sz = 1024;
gen_rand_testdata(path0, sz).unwrap();
diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs
index f2c3846..8324ade 100644
--- a/util/flashrom_tester/src/tester.rs
+++ b/util/flashrom_tester/src/tester.rs
@@ -39,9 +39,14 @@
use flashrom::FlashromError;
use flashrom::{FlashChip, Flashrom};
use serde_json::json;
+use std::fs::File;
+use std::io::Write;
use std::mem::MaybeUninit;
+use std::path::Path;
+use std::path::PathBuf;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Mutex;
+use tempdir::TempDir;
// type-signature comes from the return type of lib.rs workers.
type TestError = Box<dyn std::error::Error>;
@@ -58,23 +63,32 @@
pub wp: WriteProtectState<'a, 'static>,
/// The path to a file containing the flash contents at test start.
- // TODO(pmarheine) migrate this to a PathBuf for clarity
- original_flash_contents: String,
+ original_flash_contents: PathBuf,
/// The path to a file containing flash-sized random data
- // TODO(pmarheine) make this a PathBuf too
- random_data: String,
+ random_data: PathBuf,
+ /// The path to a file containing layout data.
+ pub layout_file: PathBuf,
+ /// A directory for temporary test data, including the above files.
+ pub tmp_dir: TempDir,
}
impl<'a> TestEnv<'a> {
- pub fn create(chip_type: FlashChip, cmd: &'a dyn Flashrom) -> Result<Self, FlashromError> {
+ pub fn create(
+ chip_type: FlashChip,
+ cmd: &'a dyn Flashrom,
+ print_layout: bool,
+ ) -> Result<Self, FlashromError> {
let rom_sz = cmd.get_size()?;
+ let tmp_dir = TempDir::new("flashrom_tester").map_err(|e| e.to_string())?;
let out = TestEnv {
chip_type,
cmd,
layout: utils::get_layout_sizes(rom_sz)?,
wp: WriteProtectState::from_hardware(cmd, chip_type)?,
- original_flash_contents: "/tmp/flashrom_tester_golden.bin".into(),
- random_data: "/tmp/random_content.bin".into(),
+ original_flash_contents: tmp_dir.path().join("flashrom_tester_golden.bin"),
+ random_data: tmp_dir.path().join("random_content.bin"),
+ layout_file: create_layout_file(rom_sz, tmp_dir.path(), print_layout),
+ tmp_dir,
};
info!("Stashing golden image for verification/recovery on completion");
@@ -113,7 +127,7 @@
/// Return the path to a file that contains random data and is the same size
/// as the flash chip.
- pub fn random_data_file(&self) -> &str {
+ pub fn random_data_file(&self) -> &Path {
&self.random_data
}
@@ -147,7 +161,7 @@
/// path.
///
/// Returns Err if they are not the same.
- pub fn verify(&self, contents_path: &str) -> Result<(), FlashromError> {
+ pub fn verify(&self, contents_path: &Path) -> Result<(), FlashromError> {
self.cmd.verify_from_file(contents_path)?;
Ok(())
}
@@ -487,17 +501,38 @@
}
}
+fn create_layout_file(rom_sz: i64, tmp_dir: &Path, print_layout: bool) -> PathBuf {
+ info!("Calculate ROM partition sizes & Create the layout file.");
+ let layout_sizes = utils::get_layout_sizes(rom_sz).expect("Could not partition rom");
+
+ let layout_file = tmp_dir.join("layout.file");
+ let mut f = File::create(&layout_file).expect("Could not create layout file");
+ let mut buf: Vec<u8> = vec![];
+ utils::construct_layout_file(&mut buf, &layout_sizes).expect("Could not construct layout file");
+
+ f.write_all(&buf).expect("Writing layout file failed");
+ if print_layout {
+ info!(
+ "Dumping layout file as requested:\n{}",
+ String::from_utf8_lossy(&buf)
+ );
+ }
+ layout_file
+}
+
pub fn run_all_tests<T, TS>(
chip: FlashChip,
cmd: &dyn Flashrom,
ts: TS,
terminate_flag: Option<&AtomicBool>,
+ print_layout: bool,
) -> Vec<(String, (TestConclusion, Option<TestError>))>
where
T: TestCase + Copy,
TS: IntoIterator<Item = T>,
{
- let mut env = TestEnv::create(chip, cmd).expect("Failed to set up test environment");
+ let mut env =
+ TestEnv::create(chip, cmd, print_layout).expect("Failed to set up test environment");
let mut results = Vec::new();
for t in ts {
diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs
index 1527a6e..94fe14d 100644
--- a/util/flashrom_tester/src/tests.rs
+++ b/util/flashrom_tester/src/tests.rs
@@ -40,12 +40,9 @@
use std::collections::{HashMap, HashSet};
use std::convert::TryInto;
use std::fs::{self, File};
-use std::io::{BufRead, Write};
+use std::io::BufRead;
use std::sync::atomic::AtomicBool;
-const LAYOUT_FILE: &str = "/tmp/layout.file";
-const ELOG_FILE: &str = "/tmp/elog.file";
-
/// Iterate over tests, yielding only those tests with names matching filter_names.
///
/// If filter_names is None, all tests will be run. None is distinct from Some(∅);
@@ -91,23 +88,6 @@
) -> Result<(), Box<dyn std::error::Error>> {
utils::ac_power_warning();
- info!("Calculate ROM partition sizes & Create the layout file.");
- let rom_sz: i64 = cmd.get_size()?;
- let layout_sizes = utils::get_layout_sizes(rom_sz)?;
- {
- let mut f = File::create(LAYOUT_FILE)?;
- let mut buf: Vec<u8> = vec![];
- utils::construct_layout_file(&mut buf, &layout_sizes)?;
-
- f.write_all(&buf)?;
- if print_layout {
- info!(
- "Dumping layout file as requested:\n{}",
- String::from_utf8_lossy(&buf)
- );
- }
- }
-
info!("Record crossystem information.\n{}", crossystem);
// Register tests to run:
@@ -144,7 +124,7 @@
// ------------------------.
// Run all the tests and collate the findings:
- let results = tester::run_all_tests(fc, cmd, tests, terminate_flag);
+ let results = tester::run_all_tests(fc, cmd, tests, terminate_flag, print_layout);
// Any leftover filtered names were specified to be run but don't exist
for leftover in filter_names.iter().flatten() {
@@ -241,15 +221,17 @@
env.ensure_golden()?;
const ELOG_RW_REGION_NAME: &str = "RW_ELOG";
+ let elog_file = env.tmp_dir.path().join("elog.file");
+
env.cmd
- .read_region_into_file(ELOG_FILE, ELOG_RW_REGION_NAME)?;
+ .read_region_into_file(&elog_file, ELOG_RW_REGION_NAME)?;
// Just checking for the magic numer
// TODO: improve this test to read the events
- if fs::metadata(ELOG_FILE)?.len() < 4 {
+ if fs::metadata(&elog_file)?.len() < 4 {
return Err("ELOG contained no data".into());
}
- let data = fs::read(ELOG_FILE)?;
+ let data = fs::read(&elog_file)?;
if u32::from_le_bytes(data[0..4].try_into()?) != 0x474f4c45 {
return Err("ELOG had bad magic number".into());
}
@@ -295,7 +277,7 @@
// Check that we cannot write to the protected region.
let rws = flashrom::ROMWriteSpecifics {
- layout_file: Some(LAYOUT_FILE),
+ layout_file: Some(&env.layout_file),
write_file: Some(env.random_data_file()),
name_file: Some(wp_section_name),
};
@@ -313,7 +295,7 @@
let (non_wp_section_name, _, _) =
utils::layout_section(env.layout(), section.get_non_overlapping_section());
let rws = flashrom::ROMWriteSpecifics {
- layout_file: Some(LAYOUT_FILE),
+ layout_file: Some(&env.layout_file),
write_file: Some(env.random_data_file()),
name_file: Some(non_wp_section_name),
};
To view, visit change 68643. To unsubscribe, or for help writing mail filters, visit settings.