Skip to content

Commit

Permalink
Use FileOpenFlags instead of OpenOptions in DynFilesystem
Browse files Browse the repository at this point in the history
Previously, DynFilesystem used OpenOptions for opening files in a custom
mode.  This has two drawbacks:  OpenOptions are constructed using a
callback leading to a rather complex method signature.  And OpenOption
depends on the Filesystem type, making it impossible to move
DynFilesystem into a separate core crate without also moving Filesystem.

This patch makes the FileOpenFlags struct public, changes its backing
type to i32 to avoid unnecessary conversions and changes the
DynFilesystem trait to accept FileOpenFlags instead of an
OpenOptionsCallback.  The affected methods are renamed from
*with_options* to *with_flags*.
  • Loading branch information
robin-nitrokey committed Aug 8, 2024
1 parent e6435d2 commit 63a1265
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
accessing `Storage`, `Filesystem` and `File` implementations for any storage.
- Added `Filesystem::mount_or_else` function ([#57][])
- Marked `Path::is_empty`, `Path::from_bytes_with_nul`, `Path::from_cstr`, `Path::from_cstr_unchecked`, `Path::as_str_ref_with_trailing_nul`, `Path::as_str`, and `PathBuf::new` as `const`.
- Made `fs::FileOpenFlags` public and added `From<fs::FileOpenFlags>` for `fs::OpenOptions`.

### Fixed

Expand All @@ -32,11 +33,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Change `Error` enum to a struct with associated constants.
- Remove `Error::Success` and enforce negative values for `Error`.
- Replace `Path::exists` with `Filesystem::exists`
- Replace `DynFilesystem::open_file_with_options_and_then{,unit}` with `DynFilesystem::open_file_with_flags_and_then{,unit}` using `FileOpenFlags` instead of `OpenOptionsCallback`

### 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`.
- Removed `object_safe::OpenOptionsCallback`.

[#47]: https://github.com/trussed-dev/littlefs2/pull/47
[#57]: https://github.com/trussed-dev/littlefs2/pull/57
Expand Down
10 changes: 8 additions & 2 deletions src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ impl Attribute {
bitflags! {
/// Definition of file open flags which can be mixed and matched as appropriate. These definitions
/// are reminiscent of the ones defined by POSIX.
struct FileOpenFlags: u32 {
pub struct FileOpenFlags: i32 {
/// Open file in read only mode.
const READ = 0x1;
/// Open file in write only mode.
Expand Down Expand Up @@ -859,7 +859,7 @@ impl OpenOptions {
&mut fs.alloc.borrow_mut().state,
addr_of_mut!(alloc.state),
path.as_ptr(),
self.0.bits() as i32,
self.0.bits(),
addr_of!(alloc.config),
);

Expand Down Expand Up @@ -950,6 +950,12 @@ impl OpenOptions {
}
}

impl From<FileOpenFlags> for OpenOptions {
fn from(flags: FileOpenFlags) -> Self {
Self(flags)
}
}

impl<S: driver::Storage> io::Read for File<'_, '_, S> {
fn read(&self, buf: &mut [u8]) -> Result<usize> {
let return_code = unsafe {
Expand Down
29 changes: 18 additions & 11 deletions src/object_safe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use heapless::Vec;

use crate::{
driver::Storage,
fs::{Attribute, DirEntry, File, Filesystem, Metadata, OpenOptions},
fs::{Attribute, DirEntry, File, FileOpenFlags, Filesystem, Metadata},
io::{Error, OpenSeekFrom, Read, Result, Seek, Write},
path::Path,
};
Expand All @@ -19,7 +19,6 @@ pub type DirEntriesCallback<'a, R = ()> =
&'a mut dyn FnMut(&mut dyn Iterator<Item = Result<DirEntry>>) -> Result<R>;
pub type FileCallback<'a, R = ()> = &'a mut dyn FnMut(&dyn DynFile) -> Result<R>;
pub type FilesystemCallback<'a, R = ()> = &'a mut dyn FnMut(&dyn DynFilesystem) -> Result<R>;
pub type OpenOptionsCallback<'a> = &'a dyn Fn(&mut OpenOptions) -> &OpenOptions;
pub type Predicate<'a> = &'a dyn Fn(&DirEntry) -> bool;

/// Object-safe trait for [`File`][].
Expand Down Expand Up @@ -75,7 +74,7 @@ impl dyn DynFile + '_ {
/// The following methods cannot support generic return types in the callbacks:
/// - [`DynFilesystem::create_file_and_then_unit`][]
/// - [`DynFilesystem::open_file_and_then_unit`][]
/// - [`DynFilesystem::open_file_with_options_and_then_unit`][]
/// - [`DynFilesystem::open_file_with_flags_and_then_unit`][]
/// - [`DynFilesystem::read_dir_and_then_unit`][]
///
/// Use these helper functions instead:
Expand All @@ -101,9 +100,9 @@ pub trait DynFilesystem {
fn metadata(&self, path: &Path) -> Result<Metadata>;
fn create_file_and_then_unit(&self, path: &Path, f: FileCallback<'_>) -> Result<()>;
fn open_file_and_then_unit(&self, path: &Path, f: FileCallback<'_>) -> Result<()>;
fn open_file_with_options_and_then_unit(
fn open_file_with_flags_and_then_unit(
&self,
o: OpenOptionsCallback<'_>,
flags: FileOpenFlags,
path: &Path,
f: FileCallback<'_>,
) -> Result<()>;
Expand Down Expand Up @@ -172,13 +171,21 @@ impl<S: Storage> DynFilesystem for Filesystem<'_, S> {
Filesystem::open_file_and_then(self, path, |file| f(file))
}

fn open_file_with_options_and_then_unit(
fn open_file_with_flags_and_then_unit(
&self,
o: OpenOptionsCallback<'_>,
flags: FileOpenFlags,
path: &Path,
f: FileCallback<'_>,
) -> Result<()> {
Filesystem::open_file_with_options_and_then(self, o, path, |file| f(file))
Filesystem::open_file_with_options_and_then(
self,
|o| {
*o = flags.into();
o
},
path,
|file| f(file),
)
}

fn attribute(&self, path: &Path, id: u8) -> Result<Option<Attribute>> {
Expand Down Expand Up @@ -257,14 +264,14 @@ impl dyn DynFilesystem + '_ {
result
}

pub fn open_file_with_options_and_then<R>(
pub fn open_file_with_flags_and_then<R>(
&self,
o: OpenOptionsCallback<'_>,
flags: FileOpenFlags,
path: &Path,
f: FileCallback<'_, R>,
) -> Result<R> {
let mut result = Err(Error::IO);
self.open_file_with_options_and_then_unit(o, path, &mut |file| {
self.open_file_with_flags_and_then_unit(flags, path, &mut |file| {
result = Ok(f(file)?);
Ok(())
})?;
Expand Down

0 comments on commit 63a1265

Please sign in to comment.