Skip to content

Commit

Permalink
Replace Error enum with struct
Browse files Browse the repository at this point in the history
To make it easier to add support for new error codes without breaking
compatibility, this patch replaces the Error enum with a struct and
associated constants.  As a part of this change, it also enforces that
Error is not used for return values indicating success, i. e. only for
negative return values.
  • Loading branch information
robin-nitrokey committed Jul 4, 2024
1 parent 9ed715b commit 41c0b3e
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 99 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@ 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:
- Change `Error` enum to a struct with associated constants.
- Remove `Error::Success` and enforce negative values error codes for `Error`.

### Removed

- Removed `Path::from_bytes_with_nul_unchecked`. Use `CStr::from_bytes_with_nul_unchecked` and `Path::from_cstr_unchecked` instead.
- Removed `From<littlefs2::path::Error> for littlefs2::io::Error`.

[#47]: https://github.com/trussed-dev/littlefs2/pull/47
[#57]: https://github.com/trussed-dev/littlefs2/pull/57
Expand Down
18 changes: 7 additions & 11 deletions src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|_| Error::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 != Error::ENTRY_ALREADY_EXISTED {
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 != Error::ENTRY_ALREADY_EXISTED {
return Err(error);
}
}
Ok(())
Expand Down
199 changes: 125 additions & 74 deletions src/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
pub mod prelude;

use core::{
ffi::c_int,
fmt::{self, Debug, Formatter},
};

use littlefs2_sys as ll;

/// The `Read` trait allows for reading bytes from a file.
Expand All @@ -17,7 +22,7 @@ pub trait Read {
Ok(())
} else {
// TODO: Decide whether to add an equivalent of `ErrorKind::UnexpectedEof`
Err(Error::Io)
Err(Error::IO)
}
}
}
Expand All @@ -43,7 +48,7 @@ pub trait Write {
match self.write(buf) {
Ok(0) => {
// failed to write whole buffer
return Err(Error::Io);
return Err(Error::IO);
}
Ok(n) => buf = &buf[n..],
Err(e) => return Err(e),
Expand Down Expand Up @@ -112,105 +117,151 @@ pub trait Seek {

pub type Result<T> = core::result::Result<T, Error>;

/// Definition of errors that might be returned by filesystem functionality.
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum Error {
/// Error code was >=0, operation was successful.
Success,
/// The error type for filesystem operations.
///
/// Specific error codes are available as associated constants of this type.
///
/// ```
/// # use littlefs2::io::Error;
///
/// assert_eq!(Error::IO.code(), -5);
/// assert_eq!(Error::new(-5), Some(Error::IO));
/// ```
#[derive(Clone, Copy, PartialEq)]
pub struct Error {
code: c_int,
}

impl Error {
/// Input / output error occurred.
Io,
pub const IO: Self = Self::new_const(ll::lfs_error_LFS_ERR_IO);

/// File or filesystem was corrupt.
Corruption,
pub const CORRUPTION: Self = Self::new_const(ll::lfs_error_LFS_ERR_CORRUPT);

/// No entry found with that name.
NoSuchEntry,
pub const NO_SUCH_ENTRY: Self = Self::new_const(ll::lfs_error_LFS_ERR_NOENT);

/// File or directory already exists.
EntryAlreadyExisted,
pub const ENTRY_ALREADY_EXISTED: Self = Self::new_const(ll::lfs_error_LFS_ERR_EXIST);

/// Path name is not a directory.
PathNotDir,
pub const PATH_NOT_DIR: Self = Self::new_const(ll::lfs_error_LFS_ERR_NOTDIR);

/// Path specification is to a directory.
PathIsDir,
pub const PATH_IS_DIR: Self = Self::new_const(ll::lfs_error_LFS_ERR_ISDIR);

/// Directory was not empty.
DirNotEmpty,
pub const DIR_NOT_EMPTY: Self = Self::new_const(ll::lfs_error_LFS_ERR_NOTEMPTY);

/// Bad file descriptor.
BadFileDescriptor,
pub const BAD_FILE_DESCRIPTOR: Self = Self::new_const(ll::lfs_error_LFS_ERR_BADF);

/// File is too big.
FileTooBig,
pub const FILE_TOO_BIG: Self = Self::new_const(ll::lfs_error_LFS_ERR_FBIG);

/// Incorrect value specified to function.
Invalid,
pub const INVALID: Self = Self::new_const(ll::lfs_error_LFS_ERR_INVAL);

/// No space left available for operation.
NoSpace,
pub const NO_SPACE: Self = Self::new_const(ll::lfs_error_LFS_ERR_NOSPC);

/// No memory available for completing request.
NoMemory,
pub const NO_MEMORY: Self = Self::new_const(ll::lfs_error_LFS_ERR_NOMEM);

/// No attribute or data available
NoAttribute,
pub const NO_ATTRIBUTE: Self = Self::new_const(ll::lfs_error_LFS_ERR_NOATTR);

/// Filename too long
FilenameTooLong,
/// Unknown error occurred, integer code specified.
Unknown(i32),
}
pub const FILENAME_TOO_LONG: Self = Self::new_const(ll::lfs_error_LFS_ERR_NAMETOOLONG);

impl From<crate::path::Error> for Error {
fn from(_error: crate::path::Error) -> Self {
Error::Io
/// Construct an `Error` from an error code.
///
/// Return values that are greater or equals to zero represent success. In this case, `None`
/// is returned.
pub const fn new(code: c_int) -> Option<Self> {
if code >= 0 {
None
} else {
Some(Self { code })
}
}
}

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,
const fn new_const(code: c_int) -> Self {
if code >= 0 {
panic!("error code must be negative");
}
Self { code }
}

/// Return the error code of this error.
pub const fn code(&self) -> c_int {
self.code
}

const fn kind(&self) -> Option<&'static str> {
Some(match *self {
Self::IO => "Io",
Self::CORRUPTION => "Corruption",
Self::NO_SUCH_ENTRY => "NoSuchEntry",
Self::ENTRY_ALREADY_EXISTED => "EntryAlreadyExisted",
Self::PATH_NOT_DIR => "PathNotDir",
Self::PATH_IS_DIR => "PathIsDir",
Self::DIR_NOT_EMPTY => "DirNotEmpty",
Self::BAD_FILE_DESCRIPTOR => "BadFileDescriptor",
Self::FILE_TOO_BIG => "FileTooBig",
Self::INVALID => "Invalid",
Self::NO_SPACE => "NoSpace",
Self::NO_MEMORY => "NoMemory",
Self::NO_ATTRIBUTE => "NoAttribute",
Self::FILENAME_TOO_LONG => "FilenameTooLong",
_ => {
return None;
}
})
}
}

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),
}
/// Prints the numeric error code and the name of the error (if known).
///
/// ```
/// # use littlefs2::io::Error;
///
/// assert_eq!(
/// &format!("{:?}", Error::IO),
/// "Error { code: -5, kind: Some(\"Io\") }",
/// );
/// assert_eq!(
/// &format!("{:?}", Error::new(-128).unwrap()),
/// "Error { code: -128, kind: None }",
/// );
/// ```
impl Debug for Error {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
// add pseudo-field kind to debug output to make errors easier to read
f.debug_struct("Error")
.field("code", &self.code)
.field("kind", &self.kind())
.finish()
}
}

impl From<Error> for c_int {
fn from(error: Error) -> Self {
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),
if let Some(error) = Error::new(error_code) {
Err(error)
} else {
Ok(return_value)
}
}
10 changes: 5 additions & 5 deletions src/object_safe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(Error::IO);
self.create_file_and_then_unit(path, &mut |file| {
result = Ok(f(file)?);
Ok(())
Expand All @@ -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(Error::IO);
self.open_file_and_then_unit(path, &mut |file| {
result = Ok(f(file)?);
Ok(())
Expand All @@ -266,7 +266,7 @@ impl dyn DynFilesystem + '_ {
path: &Path,
f: FileCallback<'_, R>,
) -> Result<R> {
let mut result = Err(Error::Io);
let mut result = Err(Error::IO);
self.open_file_with_options_and_then_unit(o, path, &mut |file| {
result = Ok(f(file)?);
Ok(())
Expand All @@ -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(Error::IO);
self.read_dir_and_then_unit(path, &mut |entries| {
result = Ok(f(entries)?);
Ok(())
Expand Down Expand Up @@ -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(Error::IO);
self.mount_and_then_unit(&mut |fs| {
result = Ok(f(fs)?);
Ok(())
Expand Down
Loading

0 comments on commit 41c0b3e

Please sign in to comment.