Split Error into Error and ErrorKind

To make it easier to add support for new error codes, this patch renames the Error enum to ErrorKind, makes it non-exhaustive and adds a new Error struct that wraps an error code and maps it to an ErrorKind instance if the error code is known.
trussed-dev · Jul 3, 2024 · 1ef30b8 · 1ef30b8
1 parent 9ed715b
commit 1ef30b8
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 82 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Replace all panicking `Path`/`PathBuf` conversions with fallible alternatives:
   - Return a `Result` from `Path::from_str_with_nul`.
   - Replace the `From<_>` implementations for `Path` and `PathBuf` with `TryFrom<_>`, except for `From<&Path> for PathBuf`.
+- Refactor error types:
+  - Rename `Error` enum to `ErrorKind` and make it non-exhaustive.
+  - Add `Error` struct that represents an error code that may or may not map to an `ErrorKind`.
 
 ### Removed
 

diff --git a/src/fs.rs b/src/fs.rs
@@ -16,7 +16,7 @@ pub type Bytes<SIZE> = generic_array::GenericArray<u8, SIZE>;
 
 use crate::{
     driver,
-    io::{self, Error, OpenSeekFrom, Result},
+    io::{self, Error, ErrorKind, OpenSeekFrom, Result},
     path::{Path, PathBuf},
 };
 
@@ -1254,23 +1254,19 @@ impl<'a, Storage: driver::Storage> Filesystem<'a, Storage> {
         let path_slice = path.as_ref().as_bytes();
         for i in 0..path_slice.len() {
             if path_slice[i] == b'/' {
-                let dir = PathBuf::try_from(&path_slice[..i])?;
+                let dir = PathBuf::try_from(&path_slice[..i]).map_err(|_| ErrorKind::Io)?;
                 #[cfg(test)]
                 println!("generated PathBuf dir {:?} using i = {}", &dir, i);
-                match self.create_dir(&dir) {
-                    Ok(_) => {}
-                    Err(io::Error::EntryAlreadyExisted) => {}
-                    error => {
-                        panic!("{:?}", &error);
+                if let Err(error) = self.create_dir(&dir) {
+                    if error.kind() != Some(ErrorKind::EntryAlreadyExisted) {
+                        return Err(error);
                     }
                 }
             }
         }
-        match self.create_dir(path) {
-            Ok(_) => {}
-            Err(io::Error::EntryAlreadyExisted) => {}
-            error => {
-                panic!("{:?}", &error);
+        if let Err(error) = self.create_dir(path) {
+            if error.kind() != Some(ErrorKind::EntryAlreadyExisted) {
+                return Err(error);
             }
         }
         Ok(())

diff --git a/src/io.rs b/src/io.rs
@@ -2,6 +2,8 @@
 
 pub mod prelude;
 
+use core::ffi::c_int;
+
 use littlefs2_sys as ll;
 
 /// The `Read` trait allows for reading bytes from a file.
@@ -17,7 +19,7 @@ pub trait Read {
             Ok(())
         } else {
             // TODO: Decide whether to add an equivalent of `ErrorKind::UnexpectedEof`
-            Err(Error::Io)
+            Err(ErrorKind::Io.into())
         }
     }
 }
@@ -43,7 +45,7 @@ pub trait Write {
             match self.write(buf) {
                 Ok(0) => {
                     // failed to write whole buffer
-                    return Err(Error::Io);
+                    return Err(ErrorKind::Io.into());
                 }
                 Ok(n) => buf = &buf[n..],
                 Err(e) => return Err(e),
@@ -112,9 +114,95 @@ pub trait Seek {
 
 pub type Result<T> = core::result::Result<T, Error>;
 
+/// The error type for filesystem operations.
+///
+/// Specific error codes are defined in [`ErrorKind`][] and can be accessed with [`Error::kind`][].
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub struct Error {
+    code: c_int,
+}
+
+impl Error {
+    /// Construct an `Error` from an error code.
+    pub const fn new(code: c_int) -> Self {
+        Self { code }
+    }
+
+    /// Construct an `Error` from an `ErrorKind`.
+    pub const fn from_kind(kind: ErrorKind) -> Self {
+        let code = match kind {
+            ErrorKind::Success => ll::lfs_error_LFS_ERR_OK,
+            ErrorKind::Io => ll::lfs_error_LFS_ERR_IO,
+            ErrorKind::Corruption => ll::lfs_error_LFS_ERR_CORRUPT,
+            ErrorKind::NoSuchEntry => ll::lfs_error_LFS_ERR_NOENT,
+            ErrorKind::EntryAlreadyExisted => ll::lfs_error_LFS_ERR_EXIST,
+            ErrorKind::PathNotDir => ll::lfs_error_LFS_ERR_NOTDIR,
+            ErrorKind::PathIsDir => ll::lfs_error_LFS_ERR_ISDIR,
+            ErrorKind::DirNotEmpty => ll::lfs_error_LFS_ERR_NOTEMPTY,
+            ErrorKind::BadFileDescriptor => ll::lfs_error_LFS_ERR_BADF,
+            ErrorKind::FileTooBig => ll::lfs_error_LFS_ERR_FBIG,
+            ErrorKind::Invalid => ll::lfs_error_LFS_ERR_INVAL,
+            ErrorKind::NoSpace => ll::lfs_error_LFS_ERR_NOSPC,
+            ErrorKind::NoMemory => ll::lfs_error_LFS_ERR_NOMEM,
+            ErrorKind::NoAttribute => ll::lfs_error_LFS_ERR_NOATTR,
+            ErrorKind::FilenameTooLong => ll::lfs_error_LFS_ERR_NAMETOOLONG,
+        };
+        Self::new(code)
+    }
+
+    /// Return the error code of this error.
+    pub const fn code(&self) -> c_int {
+        self.code
+    }
+
+    /// Return the `ErrorKind` of this error.
+    ///
+    /// If the error code is unknown, `None` is returned.  As `ErrorKind` is a non-exhaustive enum,
+    /// adding variants is not considered a breaking change.  Therefore, users must not rely on
+    /// this method returning `None`.
+    pub const fn kind(&self) -> Option<ErrorKind> {
+        let kind = match self.code {
+            n if n >= 0 => ErrorKind::Success,
+            // negative codes
+            ll::lfs_error_LFS_ERR_IO => ErrorKind::Io,
+            ll::lfs_error_LFS_ERR_CORRUPT => ErrorKind::Corruption,
+            ll::lfs_error_LFS_ERR_NOENT => ErrorKind::NoSuchEntry,
+            ll::lfs_error_LFS_ERR_EXIST => ErrorKind::EntryAlreadyExisted,
+            ll::lfs_error_LFS_ERR_NOTDIR => ErrorKind::PathNotDir,
+            ll::lfs_error_LFS_ERR_ISDIR => ErrorKind::PathIsDir,
+            ll::lfs_error_LFS_ERR_NOTEMPTY => ErrorKind::DirNotEmpty,
+            ll::lfs_error_LFS_ERR_BADF => ErrorKind::BadFileDescriptor,
+            ll::lfs_error_LFS_ERR_FBIG => ErrorKind::FileTooBig,
+            ll::lfs_error_LFS_ERR_INVAL => ErrorKind::Invalid,
+            ll::lfs_error_LFS_ERR_NOSPC => ErrorKind::NoSpace,
+            ll::lfs_error_LFS_ERR_NOMEM => ErrorKind::NoMemory,
+            ll::lfs_error_LFS_ERR_NOATTR => ErrorKind::NoAttribute,
+            ll::lfs_error_LFS_ERR_NAMETOOLONG => ErrorKind::FilenameTooLong,
+            // positive codes should always indicate success
+            _ => {
+                return None;
+            }
+        };
+        Some(kind)
+    }
+}
+
+impl From<ErrorKind> for Error {
+    fn from(kind: ErrorKind) -> Self {
+        Self::from_kind(kind)
+    }
+}
+
+impl From<Error> for c_int {
+    fn from(error: Error) -> Self {
+        error.code
+    }
+}
+
 /// Definition of errors that might be returned by filesystem functionality.
 #[derive(Clone, Copy, Debug, PartialEq)]
-pub enum Error {
+#[non_exhaustive]
+pub enum ErrorKind {
     /// Error code was >=0, operation was successful.
     Success,
     /// Input / output error occurred.
@@ -145,72 +233,19 @@ pub enum Error {
     NoAttribute,
     /// Filename too long
     FilenameTooLong,
-    /// Unknown error occurred, integer code specified.
-    Unknown(i32),
-}
-
-impl From<crate::path::Error> for Error {
-    fn from(_error: crate::path::Error) -> Self {
-        Error::Io
-    }
-}
-
-impl From<Error> for ll::lfs_error {
-    fn from(error: Error) -> Self {
-        match error {
-            Error::Success => ll::lfs_error_LFS_ERR_OK,
-            Error::Io => ll::lfs_error_LFS_ERR_IO,
-            Error::Corruption => ll::lfs_error_LFS_ERR_CORRUPT,
-            Error::NoSuchEntry => ll::lfs_error_LFS_ERR_NOENT,
-            Error::EntryAlreadyExisted => ll::lfs_error_LFS_ERR_EXIST,
-            Error::PathNotDir => ll::lfs_error_LFS_ERR_NOTDIR,
-            Error::PathIsDir => ll::lfs_error_LFS_ERR_ISDIR,
-            Error::DirNotEmpty => ll::lfs_error_LFS_ERR_NOTEMPTY,
-            Error::BadFileDescriptor => ll::lfs_error_LFS_ERR_BADF,
-            Error::FileTooBig => ll::lfs_error_LFS_ERR_FBIG,
-            Error::Invalid => ll::lfs_error_LFS_ERR_INVAL,
-            Error::NoSpace => ll::lfs_error_LFS_ERR_NOSPC,
-            Error::NoMemory => ll::lfs_error_LFS_ERR_NOMEM,
-            Error::NoAttribute => ll::lfs_error_LFS_ERR_NOATTR,
-            Error::FilenameTooLong => ll::lfs_error_LFS_ERR_NAMETOOLONG,
-            Error::Unknown(error_code) => error_code,
-        }
-    }
-}
-
-impl From<i32> for Error {
-    fn from(error_code: i32) -> Error {
-        match error_code {
-            n if n >= 0 => Error::Success,
-            // negative codes
-            ll::lfs_error_LFS_ERR_IO => Error::Io,
-            ll::lfs_error_LFS_ERR_CORRUPT => Error::Corruption,
-            ll::lfs_error_LFS_ERR_NOENT => Error::NoSuchEntry,
-            ll::lfs_error_LFS_ERR_EXIST => Error::EntryAlreadyExisted,
-            ll::lfs_error_LFS_ERR_NOTDIR => Error::PathNotDir,
-            ll::lfs_error_LFS_ERR_ISDIR => Error::PathIsDir,
-            ll::lfs_error_LFS_ERR_NOTEMPTY => Error::DirNotEmpty,
-            ll::lfs_error_LFS_ERR_BADF => Error::BadFileDescriptor,
-            ll::lfs_error_LFS_ERR_FBIG => Error::FileTooBig,
-            ll::lfs_error_LFS_ERR_INVAL => Error::Invalid,
-            ll::lfs_error_LFS_ERR_NOSPC => Error::NoSpace,
-            ll::lfs_error_LFS_ERR_NOMEM => Error::NoMemory,
-            ll::lfs_error_LFS_ERR_NOATTR => Error::NoAttribute,
-            ll::lfs_error_LFS_ERR_NAMETOOLONG => Error::FilenameTooLong,
-            // positive codes should always indicate success
-            _ => Error::Unknown(error_code),
-        }
-    }
 }
 
 pub fn error_code_from<T>(result: Result<T>) -> ll::lfs_error {
-    result.err().unwrap_or(Error::Success).into()
+    result
+        .map(|_| ll::lfs_error_LFS_ERR_OK)
+        .unwrap_or_else(From::from)
 }
 
 pub fn result_from<T>(return_value: T, error_code: ll::lfs_error) -> Result<T> {
-    let error: Error = error_code.into();
-    match error {
-        Error::Success => Ok(return_value),
-        _ => Err(error),
+    let error = Error::new(error_code);
+    if error.kind() == Some(ErrorKind::Success) {
+        Ok(return_value)
+    } else {
+        Err(error)
     }
 }
diff --git a/src/object_safe.rs b/src/object_safe.rs
@@ -6,7 +6,7 @@ use heapless::Vec;
 use crate::{
     driver::Storage,
     fs::{Attribute, DirEntry, File, Filesystem, Metadata, OpenOptions},
-    io::{Error, OpenSeekFrom, Read, Result, Seek, Write},
+    io::{ErrorKind, OpenSeekFrom, Read, Result, Seek, Write},
     path::Path,
 };
 
@@ -243,7 +243,7 @@ impl dyn DynFilesystem + '_ {
     }
 
     pub fn create_file_and_then<R>(&self, path: &Path, f: FileCallback<'_, R>) -> Result<R> {
-        let mut result = Err(Error::Io);
+        let mut result = Err(ErrorKind::Io.into());
         self.create_file_and_then_unit(path, &mut |file| {
             result = Ok(f(file)?);
             Ok(())
@@ -252,7 +252,7 @@ impl dyn DynFilesystem + '_ {
     }
 
     pub fn open_file_and_then<R>(&self, path: &Path, f: FileCallback<'_, R>) -> Result<R> {
-        let mut result = Err(Error::Io);
+        let mut result = Err(ErrorKind::Io.into());
         self.open_file_and_then_unit(path, &mut |file| {
             result = Ok(f(file)?);
             Ok(())
@@ -266,7 +266,7 @@ impl dyn DynFilesystem + '_ {
         path: &Path,
         f: FileCallback<'_, R>,
     ) -> Result<R> {
-        let mut result = Err(Error::Io);
+        let mut result = Err(ErrorKind::Io.into());
         self.open_file_with_options_and_then_unit(o, path, &mut |file| {
             result = Ok(f(file)?);
             Ok(())
@@ -275,7 +275,7 @@ impl dyn DynFilesystem + '_ {
     }
 
     pub fn read_dir_and_then<R>(&self, path: &Path, f: DirEntriesCallback<'_, R>) -> Result<R> {
-        let mut result = Err(Error::Io);
+        let mut result = Err(ErrorKind::Io.into());
         self.read_dir_and_then_unit(path, &mut |entries| {
             result = Ok(f(entries)?);
             Ok(())
@@ -372,7 +372,7 @@ impl<S: Storage> DynStorage for S {
 
 impl dyn DynStorage + '_ {
     pub fn mount_and_then<R>(&mut self, f: FilesystemCallback<'_, R>) -> Result<R> {
-        let mut result = Err(Error::Io);
+        let mut result = Err(ErrorKind::Io.into());
         self.mount_and_then_unit(&mut |fs| {
             result = Ok(f(fs)?);
             Ok(())