diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f651814bc3..e73a6a8ab2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added support for non-static channels: - Added lifetimes to `ClientImplementation` and `ServiceEndpoints`. - Added the `pipe::TrussedChannel` type. +- Refactored the `Store` trait: + - Removed the requirement for a static lifetime. + - Removed the `Fs` wrapper type. + - Removed the storage types to return `&dyn DynFilesystem` instead. + - Removed the `Copy` requirement. + - Removed the `unsafe` keyword for the `Store` trait. ### Fixed diff --git a/Cargo.toml b/Cargo.toml index f179f02eae6..e34ccf2792d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -91,6 +91,7 @@ virt = ["std"] log-all = [] log-none = [] +log-trace = [] log-info = [] log-debug = [] log-warn = [] diff --git a/src/service/attest.rs b/src/service/attest.rs index ddabe475133..08e36d40598 100644 --- a/src/service/attest.rs +++ b/src/service/attest.rs @@ -439,10 +439,10 @@ impl Encodable for Name<'_> { l += 0xD; } if let Some(organization) = self.organization { - l += 11 + organization.as_bytes().len() as u16; + l += 11 + organization.len() as u16; } if let Some(state) = self.state { - l += 11 + state.as_bytes().len() as u16; + l += 11 + state.len() as u16; } Ok(l.into()) } diff --git a/src/store.rs b/src/store.rs index 43e239efafc..f9ded2046e6 100644 --- a/src/store.rs +++ b/src/store.rs @@ -72,7 +72,7 @@ //! - Alternative: subdirectory <==> RP hash, everything else in flat files //! - In any case need to "list dirs excluding . and .." or similar -use littlefs2::{driver::Storage, fs::Filesystem, path}; +use littlefs2::path; use crate::error::Error; use crate::types::{Bytes, Location}; @@ -127,39 +127,19 @@ pub mod keystore; // LfsStorage-bound types) to remove lifetimes and generic parameters from Store. // // This makes everything using it *much* more ergonomic. -pub unsafe trait Store: Copy { - type I: 'static + Storage; - type E: 'static + Storage; - type V: 'static + Storage; - fn ifs(self) -> &'static Fs; - fn efs(self) -> &'static Fs; - fn vfs(self) -> &'static Fs; +pub trait Store { + fn ifs(&self) -> &dyn DynFilesystem; + fn efs(&self) -> &dyn DynFilesystem; + fn vfs(&self) -> &dyn DynFilesystem; fn fs(&self, location: Location) -> &dyn DynFilesystem { match location { - Location::Internal => self.ifs().fs, - Location::External => self.efs().fs, - Location::Volatile => self.vfs().fs, + Location::Internal => self.ifs(), + Location::External => self.efs(), + Location::Volatile => self.vfs(), } } } -pub struct Fs { - fs: &'static Filesystem<'static, S>, -} - -impl core::ops::Deref for Fs { - type Target = Filesystem<'static, S>; - fn deref(&self) -> &Self::Target { - self.fs - } -} - -impl Fs { - pub fn new(fs: &'static Filesystem<'static, S>) -> Self { - Self { fs } - } -} - #[macro_export] macro_rules! store { ( @@ -174,19 +154,15 @@ macro_rules! store { __: core::marker::PhantomData<*mut ()>, } - unsafe impl $crate::store::Store for $store { - type I = $Ifs; - type E = $Efs; - type V = $Vfs; - - fn ifs(self) -> &'static $crate::store::Fs<$Ifs> { - unsafe { &*Self::ifs_ptr() } + impl $crate::store::Store for $store { + fn ifs(&self) -> &dyn littlefs2::object_safe::DynFilesystem { + unsafe { Self::ifs_ptr().assume_init() } } - fn efs(self) -> &'static $crate::store::Fs<$Efs> { - unsafe { &*Self::efs_ptr() } + fn efs(&self) -> &dyn littlefs2::object_safe::DynFilesystem { + unsafe { Self::efs_ptr().assume_init() } } - fn vfs(self) -> &'static $crate::store::Fs<$Vfs> { - unsafe { &*Self::vfs_ptr() } + fn vfs(&self) -> &dyn littlefs2::object_safe::DynFilesystem { + unsafe { Self::vfs_ptr().assume_init() } } } @@ -277,14 +253,9 @@ macro_rules! store { efs: &'static littlefs2::fs::Filesystem<$Efs>, vfs: &'static littlefs2::fs::Filesystem<$Vfs>, ) -> Self { - let store_ifs = $crate::store::Fs::new(ifs); - let store_efs = $crate::store::Fs::new(efs); - let store_vfs = $crate::store::Fs::new(vfs); - unsafe { - Self::ifs_ptr().write(store_ifs); - Self::efs_ptr().write(store_efs); - Self::vfs_ptr().write(store_vfs); - } + Self::ifs_ptr().write(ifs); + Self::efs_ptr().write(efs); + Self::vfs_ptr().write(vfs); Self::claim().unwrap() } @@ -317,28 +288,31 @@ macro_rules! store { } } - fn ifs_ptr() -> *mut $crate::store::Fs<$Ifs> { + fn ifs_ptr() -> &'static mut core::mem::MaybeUninit< + &'static dyn littlefs2::object_safe::DynFilesystem, + > { use core::mem::MaybeUninit; - use core::ptr::addr_of_mut; - use $crate::store::Fs; - static mut IFS: MaybeUninit> = MaybeUninit::uninit(); - unsafe { (*addr_of_mut!(IFS)).as_mut_ptr() } + static mut IFS: MaybeUninit<&'static dyn littlefs2::object_safe::DynFilesystem> = + MaybeUninit::uninit(); + unsafe { (&raw mut IFS).as_mut().unwrap() } } - fn efs_ptr() -> *mut $crate::store::Fs<$Efs> { + fn efs_ptr() -> &'static mut core::mem::MaybeUninit< + &'static dyn littlefs2::object_safe::DynFilesystem, + > { use core::mem::MaybeUninit; - use core::ptr::addr_of_mut; - use $crate::store::Fs; - static mut EFS: MaybeUninit> = MaybeUninit::uninit(); - unsafe { (*addr_of_mut!(EFS)).as_mut_ptr() } + static mut EFS: MaybeUninit<&'static dyn littlefs2::object_safe::DynFilesystem> = + MaybeUninit::uninit(); + unsafe { (&raw mut EFS).as_mut().unwrap() } } - fn vfs_ptr() -> *mut $crate::store::Fs<$Vfs> { + fn vfs_ptr() -> &'static mut core::mem::MaybeUninit< + &'static dyn littlefs2::object_safe::DynFilesystem, + > { use core::mem::MaybeUninit; - use core::ptr::addr_of_mut; - use $crate::store::Fs; - static mut VFS: MaybeUninit> = MaybeUninit::uninit(); - unsafe { (*addr_of_mut!(VFS)).as_mut_ptr() } + static mut VFS: MaybeUninit<&'static dyn littlefs2::object_safe::DynFilesystem> = + MaybeUninit::uninit(); + unsafe { (&raw mut VFS).as_mut().unwrap() } } // Ignore lint for compatibility @@ -401,8 +375,7 @@ macro_rules! store { &mut *(*addr_of_mut!(IFS_ALLOC)).as_mut_ptr(), &mut *(*addr_of_mut!(IFS_STORAGE)).as_mut_ptr(), )?); - let ifs = $crate::store::Fs::new((*addr_of!(IFS)).as_ref().unwrap()); - Self::ifs_ptr().write(ifs); + Self::ifs_ptr().write(addr_of!(IFS).as_ref().unwrap().as_ref().unwrap()); (*addr_of_mut!(EFS_ALLOC)).as_mut_ptr().write(efs_alloc); (*addr_of_mut!(EFS_STORAGE)).as_mut_ptr().write(efs_storage); @@ -410,8 +383,7 @@ macro_rules! store { &mut *(*addr_of_mut!(EFS_ALLOC)).as_mut_ptr(), &mut *(*addr_of_mut!(EFS_STORAGE)).as_mut_ptr(), )?); - let efs = $crate::store::Fs::new((*addr_of_mut!(EFS)).as_ref().unwrap()); - Self::efs_ptr().write(efs); + Self::efs_ptr().write(addr_of!(EFS).as_ref().unwrap().as_ref().unwrap()); (*addr_of_mut!(VFS_ALLOC)).as_mut_ptr().write(vfs_alloc); (*addr_of_mut!(VFS_STORAGE)).as_mut_ptr().write(vfs_storage); @@ -419,8 +391,7 @@ macro_rules! store { &mut *(*addr_of_mut!(VFS_ALLOC)).as_mut_ptr(), &mut *(*addr_of_mut!(VFS_STORAGE)).as_mut_ptr(), )?); - let vfs = $crate::store::Fs::new((*addr_of!(VFS)).as_ref().unwrap()); - Self::vfs_ptr().write(vfs); + Self::vfs_ptr().write(addr_of!(VFS).as_ref().unwrap().as_ref().unwrap()); Ok(()) } @@ -509,7 +480,7 @@ pub fn create_directories(fs: &dyn DynFilesystem, path: &Path) -> Result<(), Err /// Reads contents from path in location of store. #[inline(never)] pub fn read( - store: impl Store, + store: &impl Store, location: Location, path: &Path, ) -> Result, Error> { @@ -523,7 +494,7 @@ pub fn read( /// Writes contents to path in location of store. #[inline(never)] pub fn write( - store: impl Store, + store: &impl Store, location: Location, path: &Path, contents: &[u8], @@ -538,7 +509,7 @@ pub fn write( /// Creates parent directory if necessary, then writes. #[inline(never)] pub fn store( - store: impl Store, + store: &impl Store, location: Location, path: &Path, contents: &[u8], @@ -552,7 +523,7 @@ pub fn store( } #[inline(never)] -pub fn delete(store: impl Store, location: Location, path: &Path) -> bool { +pub fn delete(store: &impl Store, location: Location, path: &Path) -> bool { debug_now!("deleting {}", &path); let fs = store.fs(location); if fs.remove(path).is_err() { @@ -583,14 +554,14 @@ pub fn delete(store: impl Store, location: Location, path: &Path) -> bool { } #[inline(never)] -pub fn exists(store: impl Store, location: Location, path: &Path) -> bool { +pub fn exists(store: &impl Store, location: Location, path: &Path) -> bool { debug_now!("checking existence of {}", &path); store.fs(location).exists(path) } #[inline(never)] pub fn metadata( - store: impl Store, + store: &impl Store, location: Location, path: &Path, ) -> Result, Error> { @@ -603,7 +574,7 @@ pub fn metadata( } #[inline(never)] -pub fn rename(store: impl Store, location: Location, from: &Path, to: &Path) -> Result<(), Error> { +pub fn rename(store: &impl Store, location: Location, from: &Path, to: &Path) -> Result<(), Error> { debug_now!("renaming {} to {}", &from, &to); store .fs(location) @@ -612,14 +583,14 @@ pub fn rename(store: impl Store, location: Location, from: &Path, to: &Path) -> } #[inline(never)] -pub fn remove_dir(store: impl Store, location: Location, path: &Path) -> bool { +pub fn remove_dir(store: &impl Store, location: Location, path: &Path) -> bool { debug_now!("remove_dir'ing {}", &path); store.fs(location).remove_dir(path).is_ok() } #[inline(never)] pub fn remove_dir_all_where( - store: impl Store, + store: &impl Store, location: Location, path: &Path, predicate: &dyn Fn(&DirEntry) -> bool, diff --git a/src/store/certstore.rs b/src/store/certstore.rs index e37c4663a2b..69b184f25f3 100644 --- a/src/store/certstore.rs +++ b/src/store/certstore.rs @@ -31,7 +31,7 @@ impl Certstore for ClientCertstore { let locations = [Location::Internal, Location::External, Location::Volatile]; locations .iter() - .any(|&location| store::delete(self.store, location, &path)) + .any(|&location| store::delete(&self.store, location, &path)) .then_some(()) .ok_or(Error::NoSuchKey) } @@ -41,14 +41,14 @@ impl Certstore for ClientCertstore { let locations = [Location::Internal, Location::External, Location::Volatile]; locations .iter() - .find_map(|&location| store::read(self.store, location, &path).ok()) + .find_map(|&location| store::read(&self.store, location, &path).ok()) .ok_or(Error::NoSuchCertificate) } fn write_certificate(&mut self, location: Location, der: &Message) -> Result { let id = CertId::new(&mut self.rng); let path = self.cert_path(id); - store::store(self.store, location, &path, der.as_slice())?; + store::store(&self.store, location, &path, der.as_slice())?; Ok(id) } } diff --git a/src/store/counterstore.rs b/src/store/counterstore.rs index 8d5765037a7..c38e856fc2b 100644 --- a/src/store/counterstore.rs +++ b/src/store/counterstore.rs @@ -37,14 +37,14 @@ impl ClientCounterstore { fn read_counter(&mut self, location: Location, id: CounterId) -> Result { let path = self.counter_path(id); - let mut bytes: crate::Bytes<16> = store::read(self.store, location, &path)?; + let mut bytes: crate::Bytes<16> = store::read(&self.store, location, &path)?; bytes.resize_default(16).ok(); Ok(u128::from_le_bytes(bytes.as_slice().try_into().unwrap())) } fn write_counter(&mut self, location: Location, id: CounterId, value: u128) -> Result<()> { let path = self.counter_path(id); - store::store(self.store, location, &path, &value.to_le_bytes()) + store::store(&self.store, location, &path, &value.to_le_bytes()) } fn increment_location(&mut self, location: Location, id: CounterId) -> Result { diff --git a/src/store/filestore.rs b/src/store/filestore.rs index 4a813318805..cbedf19e909 100644 --- a/src/store/filestore.rs +++ b/src/store/filestore.rs @@ -286,7 +286,7 @@ impl ClientFilestore { // the client, and return both the entry and the state .map(|(i, entry)| { // The semantics is that for a non-existent file, we return None (not an error) - let data = store::read(self.store, location, entry.path()).ok(); + let data = store::read(&self.store, location, entry.path()).ok(); (i, data) // the `ok_or` dummy error followed by the `ok` in the next line is because @@ -352,7 +352,7 @@ impl ClientFilestore { }) .map(|(i, entry)| { // The semantics is that for a non-existent file, we return None (not an error) - let data = store::read(self.store, location, entry.path()).ok(); + let data = store::read(&self.store, location, entry.path()).ok(); (i, data) }) // convert Option into Result, again because `read_dir_and_then` expects this @@ -376,36 +376,36 @@ impl Filestore for ClientFilestore { fn read(&mut self, path: &Path, location: Location) -> Result> { let path = self.actual_path(path)?; - store::read(self.store, location, &path) + store::read(&self.store, location, &path) } fn write(&mut self, path: &Path, location: Location, data: &[u8]) -> Result<()> { let path = self.actual_path(path)?; - store::store(self.store, location, &path, data) + store::store(&self.store, location, &path, data) } fn exists(&mut self, path: &Path, location: Location) -> bool { if let Ok(path) = self.actual_path(path) { - store::exists(self.store, location, &path) + store::exists(&self.store, location, &path) } else { false } } fn metadata(&mut self, path: &Path, location: Location) -> Result> { let path = self.actual_path(path)?; - store::metadata(self.store, location, &path) + store::metadata(&self.store, location, &path) } fn rename(&mut self, from: &Path, to: &Path, location: Location) -> Result<()> { let from = self.actual_path(from)?; let to = self.actual_path(to)?; - store::rename(self.store, location, &from, &to) + store::rename(&self.store, location, &from, &to) } fn remove_file(&mut self, path: &Path, location: Location) -> Result<()> { let path = self.actual_path(path)?; - match store::delete(self.store, location, &path) { + match store::delete(&self.store, location, &path) { true => Ok(()), false => Err(Error::InternalError), } @@ -414,7 +414,7 @@ impl Filestore for ClientFilestore { fn remove_dir(&mut self, path: &Path, location: Location) -> Result<()> { let path = self.actual_path(path)?; - match store::delete(self.store, location, &path) { + match store::delete(&self.store, location, &path) { true => Ok(()), false => Err(Error::InternalError), } @@ -423,7 +423,7 @@ impl Filestore for ClientFilestore { fn remove_dir_all(&mut self, path: &Path, location: Location) -> Result { let path = self.actual_path(path)?; - store::remove_dir_all_where(self.store, location, &path, &|_| true) + store::remove_dir_all_where(&self.store, location, &path, &|_| true) .map_err(|_| Error::InternalError) } fn remove_dir_all_where( @@ -434,7 +434,7 @@ impl Filestore for ClientFilestore { ) -> Result { let path = self.actual_path(path)?; - store::remove_dir_all_where(self.store, location, &path, &predicate) + store::remove_dir_all_where(&self.store, location, &path, &predicate) .map_err(|_| Error::InternalError) } diff --git a/src/store/keystore.rs b/src/store/keystore.rs index 7b4f927f5c7..47417c6105e 100644 --- a/src/store/keystore.rs +++ b/src/store/keystore.rs @@ -118,7 +118,7 @@ impl Keystore for ClientKeystore { let id = self.generate_key_id(); let path = self.key_path(secrecy, &id); - store::store(self.store, location, &path, &key.serialize())?; + store::store(&self.store, location, &path, &key.serialize())?; Ok(id) } @@ -145,7 +145,7 @@ impl Keystore for ClientKeystore { locations.iter().any(|location| { secrecies.iter().any(|secrecy| { let path = self.key_path(*secrecy, id); - store::delete(self.store, *location, &path) + store::delete(&self.store, *location, &path) }) }) } @@ -159,12 +159,12 @@ impl Keystore for ClientKeystore { fn delete_all(&self, location: Location) -> Result { let secret_path = self.key_directory(key::Secrecy::Secret); let secret_deleted = - store::remove_dir_all_where(self.store, location, &secret_path, &|dir_entry| { + store::remove_dir_all_where(&self.store, location, &secret_path, &|dir_entry| { dir_entry.file_name().as_ref().len() >= 4 })?; let public_path = self.key_directory(key::Secrecy::Public); let public_deleted = - store::remove_dir_all_where(self.store, location, &public_path, &|dir_entry| { + store::remove_dir_all_where(&self.store, location, &public_path, &|dir_entry| { dir_entry.file_name().as_ref().len() >= 4 })?; Ok(secret_deleted + public_deleted) @@ -181,7 +181,7 @@ impl Keystore for ClientKeystore { let location = self.location(secrecy, id).ok_or(Error::NoSuchKey)?; - let bytes: Bytes<{ MAX_KEY_MATERIAL_LENGTH }> = store::read(self.store, location, &path)?; + let bytes: Bytes<{ MAX_KEY_MATERIAL_LENGTH }> = store::read(&self.store, location, &path)?; let key = key::Key::try_deserialize(&bytes)?; @@ -212,7 +212,7 @@ impl Keystore for ClientKeystore { }; let path = self.key_path(secrecy, id); - store::store(self.store, location, &path, &key.serialize())?; + store::store(&self.store, location, &path, &key.serialize())?; Ok(()) } diff --git a/src/virt/store.rs b/src/virt/store.rs index 25f8b69404f..5e5cd8344c4 100644 --- a/src/virt/store.rs +++ b/src/virt/store.rs @@ -16,8 +16,9 @@ use crate::{store, store::Store}; pub trait StoreProvider { type Store: Store; + type Ifs; - unsafe fn ifs() -> &'static mut ::I; + unsafe fn ifs() -> &'static mut Self::Ifs; unsafe fn store() -> Self::Store; @@ -131,6 +132,7 @@ impl Filesystem { impl StoreProvider for Filesystem { type Store = FilesystemStore; + type Ifs = FilesystemStorage; unsafe fn ifs() -> &'static mut FilesystemStorage { (*addr_of_mut!(INTERNAL_FILESYSTEM_STORAGE)) @@ -180,6 +182,7 @@ pub struct Ram {} impl StoreProvider for Ram { type Store = RamStore; + type Ifs = InternalStorage; unsafe fn ifs() -> &'static mut InternalStorage { (*addr_of_mut!(INTERNAL_RAM_STORAGE))