Change in flashrom[master]: flashrom_tester: Use Flashrom trait instead of struct FlashromCmd

[Evan Benn (Code Review)]

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/64849 )


Change subject: flashrom_tester: Use Flashrom trait instead of struct FlashromCmd
......................................................................

flashrom_tester: Use Flashrom trait instead of struct FlashromCmd

To allow FlashromCmd to be reimplemented with libflashrom move all
functions to the trait, allowing users to deal with the trait instad of
the concrete type.

Change-Id: Ie2b4e7e91d69043fd50d1c57f6585fc9946fab10
Signed-off-by: Evan Benn <[e-mail]>
---
M util/flashrom_tester/flashrom/src/cmd.rs
M util/flashrom_tester/flashrom/src/lib.rs
M util/flashrom_tester/src/tester.rs
M util/flashrom_tester/src/tests.rs
4 files changed, 311 insertions(+), 296 deletions(-)



  git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/64849/1

diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs
index 37f8a72..e80b64f 100644
--- a/util/flashrom_tester/flashrom/src/cmd.rs
+++ b/util/flashrom_tester/flashrom/src/cmd.rs
@@ -33,10 +33,39 @@
 // Software Foundation.
 //
 
-use crate::{FlashChip, FlashromError, FlashromOpt};
+use crate::{FlashChip, FlashromError, ROMWriteSpecifics};
 
 use std::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 flash_name: bool, // --flash-name
+    pub verbose: bool,    // -V
+}
+
+#[derive(Default)]
+pub struct WPOpt {
+    pub range: Option<(i64, i64)>, // --wp-range x0 x1
+    pub status: bool,              // --wp-status
+    pub list: bool,                // --wp-list
+    pub enable: bool,              // --wp-enable
+    pub disable: bool,             // --wp-disable
+}
+
+#[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
+}
+
 #[derive(PartialEq, Debug)]
 pub struct FlashromCmd {
     pub path: String,
@@ -65,6 +94,13 @@
     }
 }
 
+impl FlashromCmd {
+    fn dispatch(&self, fropt: FlashromOpt) -> Result<(Vec<u8>, Vec<u8>), FlashromError> {
+        let params = flashrom_decode_opts(fropt);
+        flashrom_dispatch(self.path.as_str(), ¶ms, self.fc)
+    }
+}
+
 impl crate::Flashrom for FlashromCmd {
     fn get_size(&self) -> Result<i64, FlashromError> {
         let (stdout, _) = flashrom_dispatch(self.path.as_str(), &["--flash-size"], self.fc)?;
@@ -73,9 +109,210 @@
         flashrom_extract_size(&sz)
     }
 
-    fn dispatch(&self, fropt: FlashromOpt) -> Result<(Vec<u8>, Vec<u8>), FlashromError> {
-        let params = flashrom_decode_opts(fropt);
-        flashrom_dispatch(self.path.as_str(), ¶ms, self.fc)
+    fn name(&self) -> Result<(String, String), FlashromError> {
+        let opts = FlashromOpt {
+            io_opt: IOOpt {
+                ..Default::default()
+            },
+
+            flash_name: true,
+
+            ..Default::default()
+        };
+
+        let (stdout, stderr) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        let eoutput = String::from_utf8_lossy(stderr.as_slice());
+        debug!("name()'stdout: {:#?}.", output);
+        debug!("name()'stderr: {:#?}.", eoutput);
+
+        match extract_flash_name(&output) {
+            None => Err("Didn't find chip vendor/name in flashrom output".into()),
+            Some((vendor, name)) => Ok((vendor.into(), name.into())),
+        }
+    }
+    fn write_file_with_layout(&self, rws: &ROMWriteSpecifics) -> Result<bool, FlashromError> {
+        let opts = FlashromOpt {
+            io_opt: IOOpt {
+                write: rws.write_file,
+                ..Default::default()
+            },
+
+            layout: rws.layout_file,
+            image: rws.name_file,
+
+            ..Default::default()
+        };
+
+        let (stdout, stderr) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        let eoutput = String::from_utf8_lossy(stderr.as_slice());
+        debug!("write_file_with_layout()'stdout:\n{}.", output);
+        debug!("write_file_with_layout()'stderr:\n{}.", eoutput);
+        Ok(true)
+    }
+    fn wp_range(&self, range: (i64, i64), wp_enable: bool) -> Result<bool, FlashromError> {
+        let opts = FlashromOpt {
+            wp_opt: WPOpt {
+                range: Some(range),
+                enable: wp_enable,
+                ..Default::default()
+            },
+            ..Default::default()
+        };
+
+        let (stdout, stderr) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        let eoutput = String::from_utf8_lossy(stderr.as_slice());
+        debug!("wp_range()'stdout:\n{}.", output);
+        debug!("wp_range()'stderr:\n{}.", eoutput);
+        Ok(true)
+    }
+
+    fn wp_list(&self) -> Result<String, FlashromError> {
+        let opts = FlashromOpt {
+            wp_opt: WPOpt {
+                list: true,
+                ..Default::default()
+            },
+            ..Default::default()
+        };
+
+        let (stdout, _) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        if output.len() == 0 {
+            return Err(
+                "wp_list isn't supported on platforms using the Linux kernel SPI driver wp_list"
+                    .into(),
+            );
+        }
+        Ok(output.to_string())
+    }
+
+    fn wp_status(&self, en: bool) -> Result<bool, FlashromError> {
+        let status = if en { "en" } else { "dis" };
+        info!("See if chip write protect is {}abled", status);
+
+        let opts = FlashromOpt {
+            wp_opt: WPOpt {
+                status: true,
+                ..Default::default()
+            },
+            ..Default::default()
+        };
+
+        let (stdout, _) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+
+        debug!("wp_status():\n{}", output);
+
+        let s = std::format!("write protect is {}abled", status);
+        Ok(output.contains(&s))
+    }
+
+    fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError> {
+        let status = if en { "en" } else { "dis" };
+
+        // For MTD, --wp-range and --wp-enable must be used simultaneously.
+        let range = if en {
+            let rom_sz: i64 = self.get_size()?;
+            Some((0, rom_sz)) // (start, len)
+        } else {
+            None
+        };
+
+        let opts = FlashromOpt {
+            wp_opt: WPOpt {
+                range: range,
+                enable: en,
+                disable: !en,
+                ..Default::default()
+            },
+            ..Default::default()
+        };
+
+        let (stdout, stderr) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        let eoutput = String::from_utf8_lossy(stderr.as_slice());
+
+        debug!("wp_toggle()'stdout:\n{}.", output);
+        debug!("wp_toggle()'stderr:\n{}.", eoutput);
+
+        match self.wp_status(true) {
+            Ok(_ret) => {
+                info!("Successfully {}abled write-protect", status);
+                Ok(true)
+            }
+            Err(e) => Err(format!("Cannot {}able write-protect: {}", status, e)),
+        }
+    }
+
+    fn read(&self, path: &str) -> Result<(), FlashromError> {
+        let opts = FlashromOpt {
+            io_opt: IOOpt {
+                read: Some(path),
+                ..Default::default()
+            },
+            ..Default::default()
+        };
+
+        let (stdout, _) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        debug!("read():\n{}", output);
+        Ok(())
+    }
+
+    fn write(&self, path: &str) -> Result<(), FlashromError> {
+        let opts = FlashromOpt {
+            io_opt: IOOpt {
+                write: Some(path),
+                ..Default::default()
+            },
+            ..Default::default()
+        };
+
+        let (stdout, _) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        debug!("write():\n{}", output);
+        Ok(())
+    }
+
+    fn verify(&self, path: &str) -> Result<(), FlashromError> {
+        let opts = FlashromOpt {
+            io_opt: IOOpt {
+                verify: Some(path),
+                ..Default::default()
+            },
+            ..Default::default()
+        };
+
+        let (stdout, _) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        debug!("verify():\n{}", output);
+        Ok(())
+    }
+
+    fn erase(&self) -> Result<(), FlashromError> {
+        let opts = FlashromOpt {
+            io_opt: IOOpt {
+                erase: true,
+                ..Default::default()
+            },
+            ..Default::default()
+        };
+
+        let (stdout, _) = self.dispatch(opts)?;
+        let output = String::from_utf8_lossy(stdout.as_slice());
+        debug!("erase():\n{}", output);
+        Ok(())
+    }
+
+    fn can_control_hw_wp(&self) -> bool {
+        self.fc.can_control_hw_wp()
+    }
+
+    fn programmer_name(&self) -> String {
+        FlashChip::to(self.fc).to_string()
     }
 }
 
@@ -209,6 +446,29 @@
     format!("{:#08X},{:#08X}", s, l).to_string()
 }
 
+/// Get a flash vendor and name from the first matching line of flashrom output.
+///
+/// The target line looks like 'vendor="foo" name="bar"', as output by flashrom --flash-name.
+/// This is usually the last line of output.
+fn extract_flash_name(stdout: &str) -> Option<(&str, &str)> {
+    for line in stdout.lines() {
+        if !line.starts_with("vendor=\"") {
+            continue;
+        }
+
+        let tail = line.trim_start_matches("vendor=\"");
+        let mut split = tail.splitn(2, "\" name=\"");
+        let vendor = split.next();
+        let name = split.next().map(|s| s.trim_end_matches('"'));
+
+        match (vendor, name) {
+            (Some(v), Some(n)) => return Some((v, n)),
+            _ => continue,
+        }
+    }
+    None
+}
+
 #[cfg(test)]
 mod tests {
     use super::flashrom_decode_opts;
diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs
index 75797d3..e255c9b 100644
--- a/util/flashrom_tester/flashrom/src/lib.rs
+++ b/util/flashrom_tester/flashrom/src/lib.rs
@@ -83,273 +83,26 @@
 
 pub type FlashromError = String;
 
-#[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 flash_name: bool, // --flash-name
-    pub verbose: bool,    // -V
-}
-
-#[derive(Default)]
-pub struct WPOpt {
-    pub range: Option<(i64, i64)>, // --wp-range x0 x1
-    pub status: bool,              // --wp-status
-    pub list: bool,                // --wp-list
-    pub enable: bool,              // --wp-enable
-    pub disable: bool,             // --wp-disable
-}
-
-#[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 trait Flashrom {
-    fn get_size(&self) -> Result<i64, FlashromError>;
-    fn dispatch(&self, fropt: FlashromOpt) -> Result<(Vec<u8>, Vec<u8>), FlashromError>;
-}
-
-pub fn name(cmd: &cmd::FlashromCmd) -> Result<(String, String), FlashromError> {
-    let opts = FlashromOpt {
-        io_opt: IOOpt {
-            ..Default::default()
-        },
-
-        flash_name: true,
-
-        ..Default::default()
-    };
-
-    let (stdout, stderr) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    let eoutput = String::from_utf8_lossy(stderr.as_slice());
-    debug!("name()'stdout: {:#?}.", output);
-    debug!("name()'stderr: {:#?}.", eoutput);
-
-    match extract_flash_name(&output) {
-        None => Err("Didn't find chip vendor/name in flashrom output".into()),
-        Some((vendor, name)) => Ok((vendor.into(), name.into())),
-    }
-}
-
-/// Get a flash vendor and name from the first matching line of flashrom output.
-///
-/// The target line looks like 'vendor="foo" name="bar"', as output by flashrom --flash-name.
-/// This is usually the last line of output.
-fn extract_flash_name(stdout: &str) -> Option<(&str, &str)> {
-    for line in stdout.lines() {
-        if !line.starts_with("vendor=\"") {
-            continue;
-        }
-
-        let tail = line.trim_start_matches("vendor=\"");
-        let mut split = tail.splitn(2, "\" name=\"");
-        let vendor = split.next();
-        let name = split.next().map(|s| s.trim_end_matches('"'));
-
-        match (vendor, name) {
-            (Some(v), Some(n)) => return Some((v, n)),
-            _ => continue,
-        }
-    }
-    None
-}
-
 pub struct ROMWriteSpecifics<'a> {
     pub layout_file: Option<&'a str>,
     pub write_file: Option<&'a str>,
     pub name_file: Option<&'a str>,
 }
 
-pub fn write_file_with_layout(
-    cmd: &cmd::FlashromCmd,
-    rws: &ROMWriteSpecifics,
-) -> Result<bool, FlashromError> {
-    let opts = FlashromOpt {
-        io_opt: IOOpt {
-            write: rws.write_file,
-            ..Default::default()
-        },
-
-        layout: rws.layout_file,
-        image: rws.name_file,
-
-        ..Default::default()
-    };
-
-    let (stdout, stderr) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    let eoutput = String::from_utf8_lossy(stderr.as_slice());
-    debug!("write_file_with_layout()'stdout:\n{}.", output);
-    debug!("write_file_with_layout()'stderr:\n{}.", eoutput);
-    Ok(true)
-}
-
-pub fn wp_range(
-    cmd: &cmd::FlashromCmd,
-    range: (i64, i64),
-    wp_enable: bool,
-) -> Result<bool, FlashromError> {
-    let opts = FlashromOpt {
-        wp_opt: WPOpt {
-            range: Some(range),
-            enable: wp_enable,
-            ..Default::default()
-        },
-        ..Default::default()
-    };
-
-    let (stdout, stderr) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    let eoutput = String::from_utf8_lossy(stderr.as_slice());
-    debug!("wp_range()'stdout:\n{}.", output);
-    debug!("wp_range()'stderr:\n{}.", eoutput);
-    Ok(true)
-}
-
-pub fn wp_list(cmd: &cmd::FlashromCmd) -> Result<String, FlashromError> {
-    let opts = FlashromOpt {
-        wp_opt: WPOpt {
-            list: true,
-            ..Default::default()
-        },
-        ..Default::default()
-    };
-
-    let (stdout, _) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    if output.len() == 0 {
-        return Err(
-            "wp_list isn't supported on platforms using the Linux kernel SPI driver wp_list".into(),
-        );
-    }
-    Ok(output.to_string())
-}
-
-pub fn wp_status(cmd: &cmd::FlashromCmd, en: bool) -> Result<bool, FlashromError> {
-    let status = if en { "en" } else { "dis" };
-    info!("See if chip write protect is {}abled", status);
-
-    let opts = FlashromOpt {
-        wp_opt: WPOpt {
-            status: true,
-            ..Default::default()
-        },
-        ..Default::default()
-    };
-
-    let (stdout, _) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-
-    debug!("wp_status():\n{}", output);
-
-    let s = std::format!("write protect is {}abled", status);
-    Ok(output.contains(&s))
-}
-
-pub fn wp_toggle(cmd: &cmd::FlashromCmd, en: bool) -> Result<bool, FlashromError> {
-    let status = if en { "en" } else { "dis" };
-
-    // For MTD, --wp-range and --wp-enable must be used simultaneously.
-    let range = if en {
-        let rom_sz: i64 = cmd.get_size()?;
-        Some((0, rom_sz)) // (start, len)
-    } else {
-        None
-    };
-
-    let opts = FlashromOpt {
-        wp_opt: WPOpt {
-            range: range,
-            enable: en,
-            disable: !en,
-            ..Default::default()
-        },
-        ..Default::default()
-    };
-
-    let (stdout, stderr) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    let eoutput = String::from_utf8_lossy(stderr.as_slice());
-
-    debug!("wp_toggle()'stdout:\n{}.", output);
-    debug!("wp_toggle()'stderr:\n{}.", eoutput);
-
-    match wp_status(&cmd, true) {
-        Ok(_ret) => {
-            info!("Successfully {}abled write-protect", status);
-            Ok(true)
-        }
-        Err(e) => Err(format!("Cannot {}able write-protect: {}", status, e)),
-    }
-}
-
-pub fn read(cmd: &cmd::FlashromCmd, path: &str) -> Result<(), FlashromError> {
-    let opts = FlashromOpt {
-        io_opt: IOOpt {
-            read: Some(path),
-            ..Default::default()
-        },
-        ..Default::default()
-    };
-
-    let (stdout, _) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    debug!("read():\n{}", output);
-    Ok(())
-}
-
-pub fn write(cmd: &cmd::FlashromCmd, path: &str) -> Result<(), FlashromError> {
-    let opts = FlashromOpt {
-        io_opt: IOOpt {
-            write: Some(path),
-            ..Default::default()
-        },
-        ..Default::default()
-    };
-
-    let (stdout, _) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    debug!("write():\n{}", output);
-    Ok(())
-}
-
-pub fn verify(cmd: &cmd::FlashromCmd, path: &str) -> Result<(), FlashromError> {
-    let opts = FlashromOpt {
-        io_opt: IOOpt {
-            verify: Some(path),
-            ..Default::default()
-        },
-        ..Default::default()
-    };
-
-    let (stdout, _) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    debug!("verify():\n{}", output);
-    Ok(())
-}
-
-pub fn erase(cmd: &cmd::FlashromCmd) -> Result<(), FlashromError> {
-    let opts = FlashromOpt {
-        io_opt: IOOpt {
-            erase: true,
-            ..Default::default()
-        },
-        ..Default::default()
-    };
-
-    let (stdout, _) = cmd.dispatch(opts)?;
-    let output = String::from_utf8_lossy(stdout.as_slice());
-    debug!("erase():\n{}", output);
-    Ok(())
+pub trait Flashrom {
+    fn get_size(&self) -> Result<i64, FlashromError>;
+    fn name(&self) -> Result<(String, String), FlashromError>;
+    fn write_file_with_layout(&self, rws: &ROMWriteSpecifics) -> Result<bool, FlashromError>;
+    fn wp_range(&self, range: (i64, i64), wp_enable: bool) -> Result<bool, FlashromError>;
+    fn wp_list(&self) -> Result<String, FlashromError>;
+    fn wp_status(&self, en: bool) -> Result<bool, FlashromError>;
+    fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError>;
+    fn read(&self, path: &str) -> Result<(), FlashromError>;
+    fn write(&self, path: &str) -> Result<(), FlashromError>;
+    fn verify(&self, path: &str) -> Result<(), FlashromError>;
+    fn erase(&self) -> Result<(), FlashromError>;
+    fn can_control_hw_wp(&self) -> bool;
+    fn programmer_name(&self) -> String;
 }
 
 #[cfg(test)]
diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs
index 3150a43..452f791 100644
--- a/util/flashrom_tester/src/tester.rs
+++ b/util/flashrom_tester/src/tester.rs
@@ -36,7 +36,7 @@
 use super::rand_util;
 use super::types;
 use super::utils::{self, LayoutSizes};
-use flashrom::{FlashChip, Flashrom, FlashromCmd};
+use flashrom::{FlashChip, Flashrom};
 use serde_json::json;
 use std::mem::MaybeUninit;
 use std::sync::atomic::{AtomicBool, Ordering};
@@ -52,7 +52,7 @@
     ///
     /// Where possible, prefer to use methods on the TestEnv rather than delegating
     /// to the raw flashrom functions.
-    pub cmd: &'a FlashromCmd,
+    pub cmd: &'a dyn Flashrom,
     layout: LayoutSizes,
 
     pub wp: WriteProtectState<'a, 'static>,
@@ -65,7 +65,7 @@
 }
 
 impl<'a> TestEnv<'a> {
-    pub fn create(chip_type: FlashChip, cmd: &'a FlashromCmd) -> Result<Self, String> {
+    pub fn create(chip_type: FlashChip, cmd: &'a impl Flashrom) -> Result<Self, String> {
         let rom_sz = cmd.get_size()?;
         let out = TestEnv {
             chip_type: chip_type,
@@ -77,8 +77,8 @@
         };
 
         info!("Stashing golden image for verification/recovery on completion");
-        flashrom::read(&out.cmd, &out.original_flash_contents)?;
-        flashrom::verify(&out.cmd, &out.original_flash_contents)?;
+        out.cmd.read(&out.original_flash_contents)?;
+        out.cmd.verify(&out.original_flash_contents)?;
 
         info!("Generating random flash-sized data");
         rand_util::gen_rand_testdata(&out.random_data, rom_sz as usize)
@@ -123,19 +123,19 @@
     /// Return true if the current Flash contents are the same as the golden image
     /// that was present at the start of testing.
     pub fn is_golden(&self) -> bool {
-        flashrom::verify(&self.cmd, &self.original_flash_contents).is_ok()
+        self.cmd.verify(&self.original_flash_contents).is_ok()
     }
 
     /// Do whatever is necessary to make the current Flash contents the same as they
     /// were at the start of testing.
     pub fn ensure_golden(&mut self) -> Result<(), String> {
         self.wp.set_hw(false)?.set_sw(false)?;
-        flashrom::write(&self.cmd, &self.original_flash_contents)
+        self.cmd.write(&self.original_flash_contents)
     }
 
     /// Attempt to erase the flash.
     pub fn erase(&self) -> Result<(), String> {
-        flashrom::erase(self.cmd)
+        self.cmd.erase()
     }
 
     /// Verify that the current Flash contents are the same as the file at the given
@@ -143,11 +143,11 @@
     ///
     /// Returns Err if they are not the same.
     pub fn verify(&self, contents_path: &str) -> Result<(), String> {
-        flashrom::verify(self.cmd, contents_path)
+        self.cmd.verify(contents_path)
     }
 }
 
-impl Drop for TestEnv<'_> {
+impl<'a> Drop for TestEnv<'a> {
     fn drop(&mut self) {
         info!("Verifying flash remains unmodified");
         if !self.is_golden() {
@@ -177,7 +177,7 @@
     initial: InitialState<'p>,
     // Tuples are (hardware, software)
     current: (bool, bool),
-    cmd: &'a FlashromCmd,
+    cmd: &'a dyn Flashrom,
 }
 
 enum InitialState<'p> {
@@ -199,7 +199,7 @@
     ///
     /// Panics if there is already a live state derived from hardware. In such a situation the
     /// new state must be derived from the live one, or the live one must be dropped first.
-    pub fn from_hardware(cmd: &'a FlashromCmd) -> Result<Self, String> {
+    pub fn from_hardware(cmd: &'a impl Flashrom) -> Result<Self, String> {
         let mut lock = Self::get_liveness_lock()
             .lock()
             .expect("Somebody panicked during WriteProtectState init from hardware");
@@ -221,8 +221,8 @@
     }
 
     /// Get the actual hardware write protect state.
-    fn get_hw(cmd: &FlashromCmd) -> Result<bool, String> {
-        if cmd.fc.can_control_hw_wp() {
+    fn get_hw(cmd: &impl Flashrom) -> Result<bool, String> {
+        if cmd.can_control_hw_wp() {
             super::utils::get_hardware_wp()
         } else {
             Ok(false)
@@ -230,8 +230,8 @@
     }
 
     /// Get the actual software write protect state.
-    fn get_sw(cmd: &FlashromCmd) -> Result<bool, String> {
-        flashrom::wp_status(cmd, true)
+    fn get_sw(cmd: &impl Flashrom) -> Result<bool, String> {
+        cmd.wp_status(true)
     }
 }
 
@@ -241,14 +241,14 @@
     ///
     /// If false, calls to set_hw() will do nothing.
     pub fn can_control_hw_wp(&self) -> bool {
-        self.cmd.fc.can_control_hw_wp()
+        self.cmd.can_control_hw_wp()
     }
 
     /// Set the software write protect.
     pub fn set_sw(&mut self, enable: bool) -> Result<&mut Self, String> {
         info!("request={}, current={}", enable, self.current.1);
         if self.current.1 != enable {
-            flashrom::wp_toggle(self.cmd, /* en= */ enable)?;
+            self.cmd.wp_toggle(/* en= */ enable)?;
             self.current.1 = enable;
         }
         Ok(self)
@@ -263,7 +263,7 @@
             } else if enable {
                 info!(
                     "Ignoring attempt to enable hardware WP with {:?} programmer",
-                    self.cmd.fc
+                    self.cmd.programmer_name()
                 );
             }
         }
@@ -376,7 +376,7 @@
                     )
                 })?;
             }
-            flashrom::wp_toggle(self.cmd, /* en= */ sw).map_err(|e| {
+            self.cmd.wp_toggle(/* en= */ sw).map_err(|e| {
                 format!(
                     "Failed to {}able software write protect: {}",
                     enable_str(sw),
@@ -386,7 +386,7 @@
         }
 
         assert!(
-            self.cmd.fc.can_control_hw_wp() || (!self.current.0 && !hw),
+            self.cmd.can_control_hw_wp() || (!self.current.0 && !hw),
             "HW WP must be disabled if it cannot be controlled"
         );
         if hw != self.current.0 {
@@ -479,7 +479,7 @@
 
 pub fn run_all_tests<T, TS>(
     chip: FlashChip,
-    cmd: &FlashromCmd,
+    cmd: impl Flashrom,
     ts: TS,
     terminate_flag: Option<&AtomicBool>,
 ) -> Vec<(String, (TestConclusion, Option<TestError>))>
@@ -487,7 +487,7 @@
     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).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 86c0da5..e9cf8a5 100644
--- a/util/flashrom_tester/src/tests.rs
+++ b/util/flashrom_tester/src/tests.rs
@@ -142,18 +142,20 @@
     };
     let tests = filter_tests(tests, &mut filter_names);
 
+    let chip_name = cmd
+        .name()
+        .map(|x| format!("vendor=\"{}\" name=\"{}\"", x.0, x.1))
+        .unwrap_or("<Unknown chip>".into());
+
     // ------------------------.
     // 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);
 
     // Any leftover filtered names were specified to be run but don't exist
     for leftover in filter_names.iter().flatten() {
         warn!("No test matches filter name \"{}\"", leftover);
     }
 
-    let chip_name = flashrom::name(&cmd)
-        .map(|x| format!("vendor=\"{}\" name=\"{}\"", x.0, x.1))
-        .unwrap_or("<Unknown chip>".into());
     let os_rel = sys_info::os_release().unwrap_or("<Unknown OS>".to_string());
     let system_info = cros_sysinfo::system_info().unwrap_or("<Unknown System>".to_string());
     let bios_info = cros_sysinfo::bios_info().unwrap_or("<Unknown BIOS>".to_string());
@@ -170,7 +172,7 @@
 
 fn get_device_name_test(env: &mut TestEnv) -> TestResult {
     // Success means we got something back, which is good enough.
-    flashrom::name(env.cmd)?;
+    env.cmd.name()?;
     Ok(())
 }
 
@@ -178,7 +180,7 @@
     // NOTE: This is not strictly a 'test' as it is allowed to fail on some platforms.
     //       However, we will warn when it does fail.
     // List the write-protected regions of flash.
-    match flashrom::wp_list(env.cmd) {
+    match env.cmd.wp_list() {
         Ok(list_str) => info!("\n{}", list_str),
         Err(e) => warn!("{}", e),
     };
@@ -289,7 +291,7 @@
         // Disable software WP so we can do range protection, but hardware WP
         // must remain enabled for (most) range protection to do anything.
         env.wp.set_hw(false)?.set_sw(false)?;
-        flashrom::wp_range(env.cmd, (start, len), true)?;
+        env.cmd.wp_range((start, len), true)?;
         env.wp.set_hw(true)?;
 
         let rws = flashrom::ROMWriteSpecifics {
@@ -297,7 +299,7 @@
             write_file: Some(env.random_data_file()),
             name_file: Some(name),
         };
-        if flashrom::write_file_with_layout(env.cmd, &rws).is_ok() {
+        if env.cmd.write_file_with_layout(&rws).is_ok() {
             return Err(
                 "Section should be locked, should not have been overwritable with random data"
                     .into(),

-- 
To view, visit https://review.coreboot.org/c/flashrom/+/64849
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie2b4e7e91d69043fd50d1c57f6585fc9946fab10
Gerrit-Change-Number: 64849
Gerrit-PatchSet: 1
Gerrit-Owner: Evan Benn <[e-mail]>
Gerrit-MessageType: newchange