Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider switching paths to use native slices #48

Open
sosthene-nitrokey opened this issue Nov 15, 2023 · 6 comments
Open

Consider switching paths to use native slices #48

sosthene-nitrokey opened this issue Nov 15, 2023 · 6 comments

Comments

@sosthene-nitrokey
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

sosthene-nitrokey commented Apr 4, 2024

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants