From a51f5f8b930d7a12d85ddcf0f3d5076c35b1171e Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Tue, 10 Dec 2024 14:50:14 +0000 Subject: [PATCH] fix: dead code warning on ARM code the whole cfg(not(test)) stuff on `impl Default for CacheEngine` meant that rustc was complaining about `HostCacheEngine` and `readln_special` were dead code when compiling unit tests. Fix this by refactoring the code slightly to not use this cfg(not(test)) antipattern anymore. Signed-off-by: Patrick Roy --- src/vmm/src/arch/aarch64/cache_info.rs | 24 +++++++----------------- src/vmm/src/arch/aarch64/fdt.rs | 4 ++-- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/vmm/src/arch/aarch64/cache_info.rs b/src/vmm/src/arch/aarch64/cache_info.rs index cd61cabeb02..c824370cd94 100644 --- a/src/vmm/src/arch/aarch64/cache_info.rs +++ b/src/vmm/src/arch/aarch64/cache_info.rs @@ -23,7 +23,7 @@ pub(crate) enum CacheInfoError { MissingOptionalAttr(String, CacheEntry), } -struct CacheEngine { +pub struct CacheEngine { store: Box, } @@ -49,15 +49,15 @@ struct HostCacheStore { cache_dir: PathBuf, } -#[cfg(not(test))] -impl Default for CacheEngine { - fn default() -> Self { +impl CacheEngine { + pub fn host() -> Self { CacheEngine { store: Box::new(HostCacheStore { cache_dir: PathBuf::from("/sys/devices/system/cpu/cpu0/cache"), }), } } + } impl CacheStore for HostCacheStore { @@ -281,16 +281,16 @@ fn append_cache_level( pub(crate) fn read_cache_config( cache_l1: &mut Vec, cache_non_l1: &mut Vec, + cache_engine: CacheEngine, ) -> Result<(), CacheInfoError> { // It is used to make sure we log warnings for missing files only for one level because // if an attribute is missing for a level for sure it will be missing for other levels too. // Also without this mechanism we would be logging the warnings for each level which pollutes // a lot the logs. let mut logged_missing_attr = false; - let engine = CacheEngine::default(); for index in 0..=MAX_CACHE_LEVEL { - match CacheEntry::from_index(index, engine.store.as_ref()) { + match CacheEntry::from_index(index, cache_engine.store.as_ref()) { Ok(cache) => { append_cache_level(cache_l1, cache_non_l1, cache); } @@ -326,16 +326,6 @@ mod tests { dummy_fs: HashMap, } - impl Default for CacheEngine { - fn default() -> Self { - CacheEngine { - store: Box::new(MockCacheStore { - dummy_fs: create_default_store(), - }), - } - } - } - impl CacheEngine { fn new(map: &HashMap) -> Self { CacheEngine { @@ -570,7 +560,7 @@ mod tests { let mut l1_caches: Vec = Vec::new(); let mut non_l1_caches: Vec = Vec::new(); // We use sysfs for extracting the cache information. - read_cache_config(&mut l1_caches, &mut non_l1_caches).unwrap(); + read_cache_config(&mut l1_caches, &mut non_l1_caches, CacheEngine::new(&create_default_store())).unwrap(); assert_eq!(l1_caches.len(), 2); assert_eq!(l1_caches.len(), 2); } diff --git a/src/vmm/src/arch/aarch64/fdt.rs b/src/vmm/src/arch/aarch64/fdt.rs index ff8c561910a..9bc48a6f80e 100644 --- a/src/vmm/src/arch/aarch64/fdt.rs +++ b/src/vmm/src/arch/aarch64/fdt.rs @@ -13,7 +13,7 @@ use vm_fdt::{Error as VmFdtError, FdtWriter, FdtWriterNode}; use vm_memory::GuestMemoryError; use super::super::{DeviceType, InitrdConfig}; -use super::cache_info::{read_cache_config, CacheEntry}; +use super::cache_info::{read_cache_config, CacheEngine, CacheEntry}; use super::gic::GICDevice; use crate::devices::acpi::vmgenid::{VmGenId, VMGENID_MEM_SIZE}; use crate::vstate::memory::{Address, GuestMemory, GuestMemoryMmap}; @@ -116,7 +116,7 @@ fn create_cpu_nodes(fdt: &mut FdtWriter, vcpu_mpidr: &[u64]) -> Result<(), FdtEr let mut l1_caches: Vec = Vec::new(); let mut non_l1_caches: Vec = Vec::new(); // We use sysfs for extracting the cache information. - read_cache_config(&mut l1_caches, &mut non_l1_caches) + read_cache_config(&mut l1_caches, &mut non_l1_caches, CacheEngine::host()) .map_err(|err| FdtError::ReadCacheInfo(err.to_string()))?; // See https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/arm/cpus.yaml.