fixup! Remove need for generic array and typenum constants

Author: sosthene-nitrokey

core/src/io.rs

@@ -111,7 +111,7 @@ pub trait Seek {
     fn seek(&self, pos: SeekFrom) -> Result<usize>;
 }
 
-pub type Result<T> = core::result::Result<T, Error>;
+pub type Result<T, E = Error> = core::result::Result<T, E>;
 
 /// The error type for filesystem operations.
 ///

src/driver.rs

@@ -1,14 +1,11 @@
 //! The `Storage`, `Read`, `Write` and `Seek` driver.
 #![allow(non_camel_case_types)]
 
-use crate::io::Result;
+use crate::io::Error;
 
 mod private {
+    pub struct NotEnoughCapacity;
     pub trait Sealed {
-        /// The maximum capacity of the buffer type.
-        /// Can be [`usize::Max`]()
-        const MAX_CAPACITY: usize;
-
         /// Returns a buffer of bytes initialized and valid. If [`set_capacity`]() was called previously,
         /// its last call defines the minimum number of valid bytes
         fn as_ptr(&self) -> *const u8;
@@ -20,8 +17,13 @@ mod private {
         /// or at initialization through [`with_capacity`](Buffer::with_capacity)
         fn current_len(&self) -> usize;
 
-        /// Can panic if `capacity` > `Self::MAX_CAPACITY`
-        fn with_len(capacity: usize) -> Self;
+        /// Atempts to set the length of the buffer to `len`
+        ///
+        /// If succeeded, the buffer obtained through the pointer operation **must** be of at least `len` bytes
+        fn set_len(&mut self, len: usize) -> Result<(), NotEnoughCapacity>;
+
+        // We could use a `Default` trait bound but it's not implemented  for all array sizes
+        fn empty() -> Self;
     }
 }
 
@@ -31,8 +33,6 @@ pub(crate) use private::Sealed;
 pub unsafe trait Buffer: private::Sealed {}
 
 impl<const N: usize> private::Sealed for [u8; N] {
-    const MAX_CAPACITY: usize = N;
-
     fn as_ptr(&self) -> *const u8 {
         <[u8]>::as_ptr(self)
     }
@@ -45,8 +45,15 @@ impl<const N: usize> private::Sealed for [u8; N] {
         N
     }
 
-    fn with_len(len: usize) -> Self {
-        assert!(len <= N);
+    fn set_len(&mut self, len: usize) -> Result<(), private::NotEnoughCapacity> {
+        if len > N {
+            Err(private::NotEnoughCapacity)
+        } else {
+            Ok(())
+        }
+    }
+
+    fn empty() -> Self {
         [0; N]
     }
 }
@@ -55,8 +62,6 @@ unsafe impl<const N: usize> Buffer for [u8; N] {}
 
 #[cfg(feature = "alloc")]
 impl private::Sealed for alloc::vec::Vec<u8> {
-    const MAX_CAPACITY: usize = usize::MAX;
-
     fn as_ptr(&self) -> *const u8 {
         <[u8]>::as_ptr(self)
     }
@@ -69,10 +74,13 @@ impl private::Sealed for alloc::vec::Vec<u8> {
         self.len()
     }
 
-    fn with_len(len: usize) -> Self {
-        let mut this = alloc::vec::Vec::with_capacity(len);
-        this.resize(len, 0);
-        this
+    fn set_len(&mut self, len: usize) -> Result<(), private::NotEnoughCapacity> {
+        self.resize(len, 0);
+        Ok(())
+    }
+
+    fn empty() -> Self {
+        Self::new()
     }
 }
 
@@ -156,13 +164,13 @@ pub trait Storage {
 
     /// Read data from the storage device.
     /// Guaranteed to be called only with bufs of length a multiple of READ_SIZE.
-    fn read(&mut self, off: usize, buf: &mut [u8]) -> Result<usize>;
+    fn read(&mut self, off: usize, buf: &mut [u8]) -> Result<usize, Error>;
     /// Write data to the storage device.
     /// Guaranteed to be called only with bufs of length a multiple of WRITE_SIZE.
-    fn write(&mut self, off: usize, data: &[u8]) -> Result<usize>;
+    fn write(&mut self, off: usize, data: &[u8]) -> Result<usize, Error>;
     /// Erase data from the storage device.
     /// Guaranteed to be called only with bufs of length a multiple of BLOCK_SIZE.
-    fn erase(&mut self, off: usize, len: usize) -> Result<usize>;
+    fn erase(&mut self, off: usize, len: usize) -> Result<usize, Error>;
     // /// Synchronize writes to the storage device.
     // fn sync(&mut self) -> Result<usize>;
 }

src/fs.rs

@@ -42,18 +42,15 @@ pub fn u32_result(return_value: i32) -> Result<u32> {
 struct Cache<Storage: driver::Storage> {
     read: UnsafeCell<Storage::CACHE_BUFFER>,
     write: UnsafeCell<Storage::CACHE_BUFFER>,
-    // lookahead: aligned::Aligned<aligned::A4, Bytes<Storage::LOOKAHEAD_SIZE>>,
     lookahead: UnsafeCell<Storage::LOOKAHEAD_BUFFER>,
-    cache_size: usize,
 }
 
 impl<S: driver::Storage> Cache<S> {
-    pub fn new(cache_size: usize, lookahead_size: usize) -> Self {
+    pub fn new() -> Self {
         Self {
-            read: UnsafeCell::new(S::CACHE_BUFFER::with_len(cache_size)),
-            write: UnsafeCell::new(S::CACHE_BUFFER::with_len(cache_size)),
-            lookahead: UnsafeCell::new(S::LOOKAHEAD_BUFFER::with_len(lookahead_size)),
-            cache_size,
+            read: UnsafeCell::new(S::CACHE_BUFFER::empty()),
+            write: UnsafeCell::new(S::CACHE_BUFFER::empty()),
+            lookahead: UnsafeCell::new(S::LOOKAHEAD_BUFFER::empty()),
         }
     }
 }
@@ -98,7 +95,7 @@ impl<Storage: driver::Storage> Allocation<Storage> {
         debug_assert!(cache_size <= block_size);
         debug_assert!(block_size % cache_size == 0);
 
-        let cache = Cache::new(cache_size as _, lookahead_size as _);
+        let cache = Cache::new();
 
         let filename_max_plus_one: u32 = crate::consts::FILENAME_MAX_PLUS_ONE;
         debug_assert!(filename_max_plus_one > 1);
@@ -195,7 +192,7 @@ impl<Storage: driver::Storage> Filesystem<'_, Storage> {
 
     pub fn format(storage: &mut Storage) -> Result<()> {
         let alloc = &mut Allocation::new(storage);
-        let fs = Filesystem::new(alloc, storage);
+        let fs = Filesystem::new(alloc, storage)?;
         let mut alloc = fs.alloc.borrow_mut();
         let return_code = unsafe { ll::lfs_format(&mut alloc.state, &alloc.config) };
         result_from((), return_code)
@@ -527,17 +524,14 @@ impl<Storage: driver::Storage> Filesystem<'_, Storage> {
 /// The state of a `File`. Pre-allocate with `File::allocate`.
 pub struct FileAllocation<S: driver::Storage> {
     cache: UnsafeCell<S::CACHE_BUFFER>,
-    cache_size: usize,
     state: ll::lfs_file_t,
     config: ll::lfs_file_config,
 }
 
 impl<S: driver::Storage> FileAllocation<S> {
-    pub fn new(cache_size: usize) -> Self {
-        debug_assert!(cache_size > 0);
+    pub fn new() -> Self {
         Self {
-            cache: UnsafeCell::new(S::CACHE_BUFFER::with_len(cache_size)),
-            cache_size,
+            cache: UnsafeCell::new(S::CACHE_BUFFER::empty()),
             state: unsafe { mem::MaybeUninit::zeroed().assume_init() },
             config: unsafe { mem::MaybeUninit::zeroed().assume_init() },
         }
@@ -552,8 +546,8 @@ pub struct File<'a, 'b, S: driver::Storage> {
 }
 
 impl<'a, 'b, Storage: driver::Storage> File<'a, 'b, Storage> {
-    pub fn allocate(fs: &Filesystem<'_, Storage>) -> FileAllocation<Storage> {
-        FileAllocation::new(fs.cache_size)
+    pub fn allocate() -> FileAllocation<Storage> {
+        FileAllocation::new()
     }
 
     /// Returns a new OpenOptions object.
@@ -741,7 +735,10 @@ impl OpenOptions {
         alloc: &mut FileAllocation<S>,
         path: &Path,
     ) -> Result<File<'a, 'b, S>> {
-        assert_eq!(fs.cache_size, alloc.cache_size);
+        alloc.cache.get_mut().set_len(fs.cache_size).map_err(|_| {
+            error_now!("Buffer is not large enough for cache size of {cache_size}");
+            Error::NO_MEMORY
+        })?;
         alloc.config.buffer = alloc.cache.get_mut().as_mut_ptr() as *mut _;
         // We need to use addr_of_mut! here instead of & mut since
         // the FFI stores a copy of a pointer to the field state,
@@ -769,7 +766,7 @@ impl OpenOptions {
         path: &Path,
         f: impl FnOnce(&File<'a, '_, S>) -> Result<R>,
     ) -> Result<R> {
-        let mut alloc = File::allocate(fs); // lifetime 'c
+        let mut alloc = File::allocate(); // lifetime 'c
         let mut file = unsafe { self.open(fs, &mut alloc, path)? };
         // Q: what is the actually correct behaviour?
         // E.g. if res is Ok but closing gives an error.
@@ -1035,7 +1032,7 @@ impl<'a, Storage: driver::Storage> Filesystem<'a, Storage> {
 
 impl<'a, Storage: driver::Storage> Filesystem<'a, Storage> {
     pub fn mount(alloc: &'a mut Allocation<Storage>, storage: &'a mut Storage) -> Result<Self> {
-        let fs = Self::new(alloc, storage);
+        let fs = Self::new(alloc, storage)?;
         fs.raw_mount()?;
         Ok(fs)
     }
@@ -1049,7 +1046,7 @@ impl<'a, Storage: driver::Storage> Filesystem<'a, Storage> {
     where
         F: FnOnce(Error, &mut Storage) -> Result<()>,
     {
-        let fs = Self::new(alloc, storage);
+        let fs = Self::new(alloc, storage)?;
         if let Err(err) = fs.raw_mount() {
             f(err, fs.storage)?;
             fs.raw_mount()?;
@@ -1065,18 +1062,51 @@ impl<'a, Storage: driver::Storage> Filesystem<'a, Storage> {
     }
 
     // Not public, user should use `mount`, possibly after `format`
-    fn new(alloc: &'a mut Allocation<Storage>, storage: &'a mut Storage) -> Self {
+    fn new(alloc: &'a mut Allocation<Storage>, storage: &'a mut Storage) -> Result<Self> {
+        let cache_size = storage.cache_size();
+        let lookahead_size = storage.lookahead_size();
+
+        alloc
+            .cache
+            .read
+            .get_mut()
+            .set_len(cache_size)
+            .map_err(|_| {
+                error_now!("Buffer is not large enough for cache size of {cache_size}");
+                Error::NO_MEMORY
+            })?;
+        alloc
+            .cache
+            .write
+            .get_mut()
+            .set_len(cache_size)
+            .map_err(|_| {
+                error_now!("Buffer is not large enough for cache size of {cache_size}");
+                Error::NO_MEMORY
+            })?;
+        alloc
+            .cache
+            .lookahead
+            .get_mut()
+            .set_len(lookahead_size)
+            .map_err(|_| {
+                error_now!("Buffer is not large enough for lookahead size of {lookahead_size}");
+                Error::NO_MEMORY
+            })?;
+
         alloc.config.context = storage as *mut _ as *mut c_void;
 
         alloc.config.read_buffer = alloc.cache.read.get_mut().as_mut_ptr() as *mut c_void;
         alloc.config.prog_buffer = alloc.cache.write.get_mut().as_mut_ptr() as *mut c_void;
         alloc.config.lookahead_buffer = alloc.cache.lookahead.get_mut().as_mut_ptr() as *mut c_void;
+        alloc.config.cache_size = cache_size as _;
+        alloc.config.lookahead_size = lookahead_size as _;
 
-        Filesystem {
-            cache_size: alloc.cache.cache_size,
+        Ok(Filesystem {
+            cache_size,
             alloc: RefCell::new(alloc),
             storage,
-        }
+        })
     }
 
     /// Deconstruct `Filesystem`, intention is to allow access to