From 1ef30b83661bc1fd542c54c1aab77d088c584c2d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 3 Jul 2024 18:41:22 +0200 Subject: [PATCH] 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. --- CHANGELOG.md | 3 + src/fs.rs | 20 +++--- src/io.rs | 163 +++++++++++++++++++++++++++------------------ src/object_safe.rs | 12 ++-- 4 files changed, 116 insertions(+), 82 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e8a6522c..70f1a5ae7 100644 --- 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 index 2f8be1706..1f5834aa3 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -16,7 +16,7 @@ pub type Bytes = generic_array::GenericArray; 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 index b8ad5fdc1..c457539ed 100644 --- 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 = core::result::Result; +/// 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 { + 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 for Error { + fn from(kind: ErrorKind) -> Self { + Self::from_kind(kind) + } +} + +impl From 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 for Error { - fn from(_error: crate::path::Error) -> Self { - Error::Io - } -} - -impl From 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 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(result: Result) -> 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(return_value: T, error_code: ll::lfs_error) -> Result { - 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 index 625001f34..a6969e003 100644 --- 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(&self, path: &Path, f: FileCallback<'_, R>) -> Result { - 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(&self, path: &Path, f: FileCallback<'_, R>) -> Result { - 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 { - 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(&self, path: &Path, f: DirEntriesCallback<'_, R>) -> Result { - 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 DynStorage for S { impl dyn DynStorage + '_ { pub fn mount_and_then(&mut self, f: FilesystemCallback<'_, R>) -> Result { - 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(())