littlefs2 icon indicating copy to clipboard operation
littlefs2 copied to clipboard

Extract core types into separate crate

Open robin-nitrokey opened this issue 11 months ago • 4 comments

Currently, the littlefs2 crate contains both a high-level interface and its implementation. In frameworks like Trussed, we would like clients to be able to depend only on the high-level interface, while the Trussed runtime or the runners are providing the implementation details. Therefore it would make sense to move the high-level interface into a separate crate, for example littlefs2-core.

The high-level interface would be:

  • Path/PathBuf
  • DynFile/DynFilesystem/DynStorage
  • all helper types required by those types

When working on Path/PathBuf, it would also make sense to:

  • remove panicking conversions, especially in From implementations, and replace them with fallible conversions
  • replace cstr_core::CStr with core::ffi::CStr
  • properly enforce the ASCII requirement even in Path
  • add conversions from the ascii crate and/or core::ascii

robin-nitrokey avatar Mar 28 '24 11:03 robin-nitrokey

Regarding the Error type:

I would wrap the Error enum in a wrapper type like std::io::Error and expose the current enum as #[non_exhaustive] enum ErrorKind without the Unknown variant through a fn error(&self) -> Option<ErrorKind> and a fn as_code(&self) -> u32 so that we can be sure not to have any breaking change releated to Error.

I would also avoid exposing littlefs-sys in the public core API, since non-breaking C changes can lead to breaking sys changes (adding new fields).

I would also remove public dependency on heapless for core (exposed through read_to_end) or put it behind a feature heapless-07 and add the heapless version name in the function name so that heapless can be upgraded (and future versions of heapless will have a VecView that doesn't have the need for the const-generic in this case, so we will want to upgrade).

Maybe the best is just fn read_to_end(&mut self, buffer: &mut [u8]) -> Result<usize>

sosthene-nitrokey avatar Mar 28 '24 11:03 sosthene-nitrokey

@sosthene-nitrokey What is the benefit of the wrapped error type over a non-exhaustive enum?

robin-nitrokey avatar May 17 '24 08:05 robin-nitrokey

This means that Error can contain an unknown error code without needing an Unknown variant. Otherwise an Unknownerror code that becomes known can break existing pattern matchings.

sosthene-nitrokey avatar May 17 '24 08:05 sosthene-nitrokey

Ah, right, I misread as_code as only returning unknown codes. Makes sense.

robin-nitrokey avatar May 17 '24 08:05 robin-nitrokey