Merge pull request #39 from zvonkok/rename-with-auto-refresh

Update version.rs segfault and WithAutoRefresh rename

src/cache.rs

@@ -48,22 +48,20 @@ impl fmt::Display for ConflictError {
 
 impl Error for ConflictError {}
 
-// CacheOption is an option to change some aspect of default CDI behavior.
-pub trait CacheOption {
-    fn apply(&self, cache: &mut Cache);
-}
+// CdiOption is an option to change some aspect of default CDI behavior.
+// We define the CdiOption type using a type alias, which is a Box<dyn FnOnce(&mut Cache)>.
+// This means that CdiOption is a trait object that represents a one-time closure that takes a &mut Cache parameter.
+pub type CdiOption = Box<dyn FnOnce(&mut Cache)>;
 
-// WithAutoRefresh returns an option to control automatic Cache refresh.
+// with_auto_refresh returns an option to control automatic Cache refresh.
 // By default auto-refresh is enabled, the list of Spec directories are
 // monitored and the Cache is automatically refreshed whenever a change
 // is detected. This option can be used to disable this behavior when a
 // manually refreshed mode is preferable.
-pub struct WithAutoRefresh(pub bool);
-
-impl CacheOption for WithAutoRefresh {
-    fn apply(&self, cache: &mut Cache) {
-        cache.auto_refresh = self.0;
-    }
+pub fn with_auto_refresh(auto_refresh: bool) -> CdiOption {
+    Box::new(move |c: &mut Cache| {
+        c.auto_refresh = auto_refresh;
+    })
 }
 
 #[allow(dead_code)]
@@ -79,7 +77,7 @@ pub struct Cache {
     //watch: Watch,
 }
 
-pub fn new_cache(options: Vec<Arc<dyn CacheOption>>) -> Arc<Mutex<Cache>> {
+pub fn new_cache(options: Vec<CdiOption>) -> Arc<Mutex<Cache>> {
     let cache = Arc::new(Mutex::new(Cache::default()));
 
     {
@@ -110,9 +108,9 @@ impl Cache {
         }
     }
 
-    pub fn configure(&mut self, options: Vec<Arc<dyn CacheOption>>) {
+    pub fn configure(&mut self, options: Vec<CdiOption>) {
         for option in options {
-            option.apply(self);
+            option(self);
         }
     }
 
@@ -268,10 +266,11 @@ impl Cache {
             if let Some(dev) = self.devices.get(&device) {
                 let mut spec = dev.get_spec();
                 if specs.insert(spec.clone()) {
-                    match spec.edits() {
-                        Some(ce) => edits.append(ce)?,
-                        None => continue,
-                    };
+                    // spec.edits may be none when we only have dev.edits
+                    // allow dev.edits to be added even if spec.edits is None
+                    if let Some(ce) = spec.edits() {
+                        edits.append(ce)?
+                    }
                 }
                 edits.append(dev.edits())?;
             } else {

src/default_cache.rs

@@ -6,23 +6,23 @@ use std::sync::{Arc, Mutex};
 use oci_spec::runtime::Spec;
 use once_cell::sync::OnceCell;
 
-use crate::cache::{new_cache, Cache, CacheOption, WithAutoRefresh};
+use crate::cache::{new_cache, with_auto_refresh, Cache, CdiOption};
 
-fn get_or_create_default_cache(_options: Vec<Arc<dyn CacheOption>>) -> Arc<Mutex<Cache>> {
+fn get_or_create_default_cache(_options: &[CdiOption]) -> Arc<Mutex<Cache>> {
     let mut cache: OnceCell<Arc<Mutex<Cache>>> = OnceCell::new();
     cache.get_or_init(|| {
-        let options: Vec<Arc<dyn CacheOption>> = vec![Arc::new(WithAutoRefresh(true))];
+        let options: Vec<CdiOption> = vec![with_auto_refresh(true)];
         new_cache(options)
     });
     cache.take().unwrap()
 }
 
 pub fn get_default_cache() -> Arc<Mutex<Cache>> {
-    get_or_create_default_cache(vec![])
+    get_or_create_default_cache(vec![].as_ref())
 }
 
-pub fn configure(options: Vec<Arc<dyn CacheOption>>) -> Result<()> {
-    let cache = get_or_create_default_cache(options.clone());
+pub fn configure(options: Vec<CdiOption>) -> Result<()> {
+    let cache = get_or_create_default_cache(&options);
     let mut cache = cache.lock().unwrap();
     if options.is_empty() {
         return Ok(());

src/spec_dirs.rs

@@ -9,7 +9,7 @@ use lazy_static::lazy_static;
 use path_clean::clean;
 
 use crate::{
-    cache::Cache,
+    cache::{Cache, CdiOption},
     spec::{read_spec, Spec},
     utils::is_cdi_spec,
 };
@@ -32,11 +32,6 @@ lazy_static! {
     ];
 }
 
-// CdiOption is an option to change some aspect of default CDI behavior.
-// We define the CdiOption type using a type alias, which is a Box<dyn FnOnce(&mut Cache)>.
-// This means that CdiOption is a trait object that represents a one-time closure that takes a &mut Cache parameter.
-type CdiOption = Box<dyn FnOnce(&mut Cache)>;
-
 #[derive(Debug)]
 pub struct SpecError {
     message: String,

src/version.rs

@@ -165,7 +165,7 @@ fn requires_v040(spec: &CDISpec) -> bool {
     spec.devices
         .iter()
         .map(|d| &d.container_edits)
-        .chain(std::iter::once(spec.container_edits.as_ref().unwrap()))
+        .chain(spec.container_edits.as_ref())
         .flat_map(|edits| edits.mounts.iter().flat_map(|mounts| mounts.iter()))
         .any(|mount| mount.r#type.as_ref().map_or(false, |typ| !typ.is_empty()))
 }