Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Store trait #195

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ virt = ["std"]

log-all = []
log-none = []
log-trace = []
log-info = []
log-debug = []
log-warn = []
Expand Down
4 changes: 2 additions & 2 deletions src/service/attest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
125 changes: 48 additions & 77 deletions src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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<Self::I>;
fn efs(self) -> &'static Fs<Self::E>;
fn vfs(self) -> &'static Fs<Self::V>;
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<S: 'static + Storage> {
fs: &'static Filesystem<'static, S>,
}

impl<S: 'static + Storage> core::ops::Deref for Fs<S> {
type Target = Filesystem<'static, S>;
fn deref(&self) -> &Self::Target {
self.fs
}
}

impl<S: 'static + Storage> Fs<S> {
pub fn new(fs: &'static Filesystem<'static, S>) -> Self {
Self { fs }
}
}

#[macro_export]
macro_rules! store {
(
Expand All @@ -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() }
}
}

Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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<Fs<$Ifs>> = 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<Fs<$Efs>> = 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<Fs<$Vfs>> = 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
Expand Down Expand Up @@ -401,26 +375,23 @@ 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);
EFS = Some(Filesystem::mount(
&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);
VFS = Some(Filesystem::mount(
&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(())
}
Expand Down Expand Up @@ -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<const N: usize>(
store: impl Store,
store: &impl Store,
location: Location,
path: &Path,
) -> Result<Bytes<N>, Error> {
Expand All @@ -523,7 +494,7 @@ pub fn read<const N: usize>(
/// 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],
Expand All @@ -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],
Expand All @@ -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() {
Expand Down Expand Up @@ -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<Option<Metadata>, Error> {
Expand All @@ -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)
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions src/store/certstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl<S: Store> Certstore for ClientCertstore<S> {
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)
}
Expand All @@ -41,14 +41,14 @@ impl<S: Store> Certstore for ClientCertstore<S> {
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<CertId> {
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)
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/store/counterstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@ impl<S: Store> ClientCounterstore<S> {

fn read_counter(&mut self, location: Location, id: CounterId) -> Result<Counter> {
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<Counter> {
Expand Down
Loading
Loading