littlefs2 icon indicating copy to clipboard operation
littlefs2 copied to clipboard

Consider switching paths to use native slices

Open sosthene-nitrokey opened this issue 1 year ago • 6 comments

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.

sosthene-nitrokey avatar Nov 15 '23 10:11 sosthene-nitrokey

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 avatar Dec 12 '23 10:12 robin-nitrokey

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.

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

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.

sosthene-nitrokey avatar Apr 04 '24 09:04 sosthene-nitrokey

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.

robin-nitrokey avatar Apr 04 '24 09:04 robin-nitrokey

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.

sosthene-nitrokey avatar Apr 04 '24 09:04 sosthene-nitrokey

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.

robin-nitrokey avatar Apr 04 '24 10:04 robin-nitrokey