ovmm_entry: Remove need for unsafe in tests

openvmm/openvmm_entry/src/cli_args.rs

@@ -1026,6 +1026,23 @@ fn parse_path_and_len(arg: &str) -> anyhow::Result<(PathBuf, Option<u64>)> {
     })
 }
 
+impl DiskCliKind {
+    /// Parse an `autocache:[key]:<kind>` disk spec, given the cache path
+    /// (normally read from `OPENVMM_AUTO_CACHE_PATH`).
+    fn parse_autocache(
+        arg: &str,
+        cache_path: Result<String, std::env::VarError>,
+    ) -> anyhow::Result<Self> {
+        let (key, kind) = arg.split_once(':').context("expected [key]:kind")?;
+        let cache_path = cache_path.context("must set cache path via OPENVMM_AUTO_CACHE_PATH")?;
+        Ok(DiskCliKind::AutoCacheSqlite {
+            cache_path,
+            key: (!key.is_empty()).then(|| key.to_string()),
+            disk: Box::new(kind.parse()?),
+        })
+    }
+}
+
 impl FromStr for DiskCliKind {
     type Err = anyhow::Error;
 
@@ -1072,14 +1089,7 @@ impl FromStr for DiskCliKind {
                     }
                 }
                 "autocache" => {
-                    let (key, kind) = arg.split_once(':').context("expected [key]:kind")?;
-                    let cache_path = std::env::var("OPENVMM_AUTO_CACHE_PATH")
-                        .context("must set cache path via OPENVMM_AUTO_CACHE_PATH")?;
-                    DiskCliKind::AutoCacheSqlite {
-                        cache_path,
-                        key: (!key.is_empty()).then(|| key.to_string()),
-                        disk: Box::new(kind.parse()?),
-                    }
+                    Self::parse_autocache(arg, std::env::var("OPENVMM_AUTO_CACHE_PATH"))?
                 }
                 "prwrap" => DiskCliKind::PersistentReservationsWrapper(Box::new(arg.parse()?)),
                 "file" => {
@@ -2014,29 +2024,9 @@ impl FromStr for VhostUserCli {
 }
 
 #[cfg(test)]
-// UNSAFETY: Needed to set and remove environment variables in tests
-#[expect(unsafe_code)]
 mod tests {
     use super::*;
 
-    fn with_env_var<F, R>(name: &str, value: &str, f: F) -> R
-    where
-        F: FnOnce() -> R,
-    {
-        // SAFETY:
-        // Safe in a testing context because it won't be changed concurrently
-        unsafe {
-            std::env::set_var(name, value);
-        }
-        let result = f();
-        // SAFETY:
-        // Safe in a testing context because it won't be changed concurrently
-        unsafe {
-            std::env::remove_var(name);
-        }
-        result
-    }
-
     #[test]
     fn test_parse_file_disk_with_create() {
         let s = "file:test.vhd;create=1G";
@@ -2207,10 +2197,9 @@ mod tests {
 
     #[test]
     fn test_parse_autocache_sqlite_disk() {
-        // Test with environment variable set
-        let disk = with_env_var("OPENVMM_AUTO_CACHE_PATH", "/tmp/cache", || {
-            DiskCliKind::from_str("autocache::file:disk.vhd").unwrap()
-        });
+        // Test with cache path provided
+        let disk =
+            DiskCliKind::parse_autocache(":file:disk.vhd", Ok("/tmp/cache".to_string())).unwrap();
         assert!(matches!(
             disk,
             DiskCliKind::AutoCacheSqlite {
@@ -2220,8 +2209,24 @@ mod tests {
             } if cache_path == "/tmp/cache" && key.is_none()
         ));
 
-        // Test without environment variable
-        assert!(DiskCliKind::from_str("autocache::file:disk.vhd").is_err());
+        // Test with key
+        let disk =
+            DiskCliKind::parse_autocache("mykey:file:disk.vhd", Ok("/tmp/cache".to_string()))
+                .unwrap();
+        assert!(matches!(
+            disk,
+            DiskCliKind::AutoCacheSqlite {
+                cache_path,
+                key: Some(key),
+                disk: _disk,
+            } if cache_path == "/tmp/cache" && key == "mykey"
+        ));
+
+        // Test without cache path
+        assert!(
+            DiskCliKind::parse_autocache(":file:disk.vhd", Err(std::env::VarError::NotPresent),)
+                .is_err()
+        );
     }
 
     #[test]
@@ -2242,12 +2247,10 @@ mod tests {
         assert!(DiskCliKind::from_str("sqldiff:path").is_err());
 
         // Missing OPENVMM_AUTO_CACHE_PATH for AutoCacheSqlite
-        // SAFETY:
-        // Safe in a testing context because it won't be changed concurrently
-        unsafe {
-            std::env::remove_var("OPENVMM_AUTO_CACHE_PATH");
-        }
-        assert!(DiskCliKind::from_str("autocache:key:file:disk.vhd").is_err());
+        assert!(
+            DiskCliKind::parse_autocache("key:file:disk.vhd", Err(std::env::VarError::NotPresent),)
+                .is_err()
+        );
 
         // Invalid blob kind
         assert!(DiskCliKind::from_str("blob:invalid:url").is_err());

openvmm/openvmm_entry/src/lib.rs

@@ -5,7 +5,7 @@
 //! for the worker process.
 
 #![expect(missing_docs)]
-#![cfg_attr(not(test), forbid(unsafe_code))]
+#![forbid(unsafe_code)]
 
 mod cli_args;
 mod crash_dump;