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

memmap example in README.md contains undefined behavior #37

Open
tbu- opened this issue Jun 9, 2017 · 3 comments
Open

memmap example in README.md contains undefined behavior #37

tbu- opened this issue Jun 9, 2017 · 3 comments

Comments

@tbu-
Copy link

tbu- commented Jun 9, 2017

I think that the example in the README.md of memmap contains a race condition which can lead to undefined behavior:

extern crate memmap;

use memmap::{Mmap, Protection};
use std::env;
use std::io;
use std::str;

fn run() -> Result<(), io::Error> {
    let mut args = env::args().skip(1);
    let input = args.next().expect("incorrect argument");

    let map = Mmap::open_path(input, Protection::Read)?;
    unsafe {
        let all_bytes = map.as_slice();
        if let Ok(file_str) = str::from_utf8(all_bytes) {
            println!("{}", file_str);
        } else {
            println!("not utf8");
        }
    }
    Ok(())
}

If the file changes after the UTF-8 check, the program prints a &str that contains non-UTF-8 bytes.

@brson
Copy link
Owner

brson commented Jun 9, 2017

Yes, that's correct, and why it's unsafe. I don't think there's any way to avoid that potential with the shared resource of a file though, right? Probably the text should indicate this aspect of memory mapped files.

@tbu-
Copy link
Author

tbu- commented Jun 9, 2017

Probably the text should indicate this aspect of memory mapped files.

Yes, I think this needs a pretty big warning. Usually, I would assume that code in unsafe blocks does not invoke undefined behavior even though the compiler can't prove it.

I don't think there's any way to avoid that potential with the shared resource of a file though, right?

If there's no way to use the interface in a safe way, maybe it shouldn't be included in an extended standard library. The C function gets is also such a function that can't be used in a safe way, and it finally got removed after two standards, in C11.

@brson
Copy link
Owner

brson commented Jul 8, 2017

I understand the perspective, but I do see memmap as something of a fundamental building block, and a tricky thing to get right cross-platform that I would rather people use an existing crate for. For now, I've at least added some color to the description about the futility of making memory mapping safe.

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