trussed-dev / littlefs2

Idiomatic Rust API for littlefs
https://lib.rs/littlefs2
Apache License 2.0
55 stars 27 forks source link

Consider switching paths to use native slices #48

Open sosthene-nitrokey opened 11 months ago

sosthene-nitrokey commented 11 months ago

The null-terminated C strings used in Littlefs2 are annoying to use and lead to inefficient iteration and higher stack usage because of the lack of ability to slice, which means copying to buffer is often required.

The bindings could instead create a stack buffer and copy rust slices to them when required just before the call to the C bindings. This would make manipulation of path much more ergonomic.

robin-nitrokey commented 10 months ago

And if we stick with CStr, we should use core::ffi::CStr to be able to benefit from c"..." string literals instead of cstr_core::CStr.

robin-nitrokey commented 6 months ago

We discussed this again and came to the conclusion that it makes more sense to have a CStr-based implementation for Path that can be used without conversion or allocation in the FFI (instead of using slices). Maybe we can add helper functions to make it easier to implement path manipulation.

sosthene-nitrokey commented 6 months ago

One issue that remains (and that is currently not checked everywhere), is that path should not be longer than LFS_PATH_MAX.

And the PathBuf methods should also be fallible, and not panic.

robin-nitrokey commented 6 months ago

The question is whether we need this restriction in Path, or if it is sufficient if the PathBuf and pointer conversions fail. Though they should definitely not panic.

sosthene-nitrokey commented 6 months ago

I think we need it for path because of the methods that take a &Path.

Most path creation will either be done through the path! macro or through a PathBuf. We can add PathBuf methods that take CStr directly to avoid having to convert to Path when working with a PathBuf.

robin-nitrokey commented 6 months ago

The methods that pass &Path to littlefs use as_ptr to convert it to a pointer. We could change that method to check the length and return an error. But we can decide that during the implementation.