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

Safety comments are missing #99

Open
RalfJung opened this issue Aug 19, 2023 · 2 comments
Open

Safety comments are missing #99

RalfJung opened this issue Aug 19, 2023 · 2 comments

Comments

@RalfJung
Copy link

RalfJung commented Aug 19, 2023

mmap is an incredibly subtle topic when it comes to memory safety. Therefore, I would expect the unsafe functions in this library to really carefully document what is required to call them soundly. However, that is not the case -- functions like this don't have any safety comment at all!

I think the requirements are something like, "the caller must ensure that the file will not be mutated or truncated while the mapping exists".

@dhardy
Copy link

dhardy commented Sep 5, 2024

You should probably re-direct this comment at the fork memmap2-rs.
Also see RazrFalcon#101.

This type of safety does not appear to translate to Rust's unsafe well; e.g. this is used by fontdb::Database::make_shared_face_data, which is itself an unsafe method, but technically speaking (if I understand correctly) calling make_shared_face_data then makes usage of several other methods on that Database unsafe. Possibly this should instead be done via an unsafe variant of struct Database, but that API would be nigh-unusable.

@RalfJung
Copy link
Author

RalfJung commented Sep 5, 2024

This type of safety does not appear to translate to Rust's unsafe well;

Well, I would say it translates just fine, but the facilities provided by Linux unfortunately do not permit to ensure safety of mmap in a local way -- very global reasoning is required. In C that "just" makes it hard to reason about, in Rust it crosses the bar to "the type system cannot help you".

Thanks for the references, I will take a look at those crates!

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