Evan Benn has uploaded this change for review.
flashrom_tester: Change String types to PathBuf where appropriate
String types were being used for paths to files. Change those to PathBuf
and Path.
BUG=b:243460685
BRANCH=None
TEST=/usr/bin/flashrom_tester --flashrom_binary /usr/sbin/flashrom host
TEST=/usr/bin/flashrom_tester --libflashrom host
Change-Id: I69531bec5436a60430eae975eeab02c8835962bf
Signed-off-by: Evan Benn <evanbenn@chromium.org>
---
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
6 files changed, 97 insertions(+), 69 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/69064/1
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 292fb78..6ce3889 100644
--- a/util/flashrom_tester/src/tester.rs
+++ b/util/flashrom_tester/src/tester.rs
@@ -42,6 +42,8 @@
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;
@@ -60,13 +62,11 @@
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: String,
+ pub layout_file: PathBuf,
}
impl<'a> TestEnv<'a> {
@@ -83,7 +83,7 @@
wp: WriteProtectState::from_hardware(cmd, chip_type)?,
original_flash_contents: "/tmp/flashrom_tester_golden.bin".into(),
random_data: "/tmp/random_content.bin".into(),
- layout_file: create_layout_file(rom_sz, "/tmp/", print_layout),
+ layout_file: create_layout_file(rom_sz, Path::new("/tmp/"), print_layout),
};
info!("Stashing golden image for verification/recovery on completion");
@@ -122,7 +122,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
}
@@ -156,7 +156,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(())
}
@@ -496,12 +496,11 @@
}
}
-fn create_layout_file(rom_sz: i64, tmp_dir: &str, print_layout: bool) -> String {
+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 mut layout_file = tmp_dir.to_string();
- layout_file.push_str("/layout.file");
+ 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");
diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs
index 3e1737e..af34208 100644
--- a/util/flashrom_tester/src/tests.rs
+++ b/util/flashrom_tester/src/tests.rs
@@ -224,7 +224,7 @@
const ELOG_RW_REGION_NAME: &str = "RW_ELOG";
env.cmd
- .read_region_into_file(ELOG_FILE, ELOG_RW_REGION_NAME)?;
+ .read_region_into_file(ELOG_FILE.as_ref(), ELOG_RW_REGION_NAME)?;
// Just checking for the magic numer
// TODO: improve this test to read the events
To view, visit change 69064. To unsubscribe, or for help writing mail filters, visit settings.