mmap non-zero length #1873
mmap non-zero length #1873
Conversation
JonathanWoollett-Light
force-pushed
the
mmap-non-zero-length
branch
2 times, most recently
from
b7d0d7c to
eb489a9
Compare
asomers
left a comment
asomers
left a comment
There was a problem hiding this comment.
I think this is a good improvement. But don't forget a CHANGELOG. Also, what about
mremap, and munmap? The man pages have the same restriction for them. And for msync, a zero-value for the length argument has special meaning, so we should probably turn it into Option<usize>.
| /// # use nix::libc::size_t; | ||
| /// # use nix::sys::mman::{mmap, mprotect, MapFlags, ProtFlags}; | ||
| /// # use std::ptr; | ||
| /// const ONE_K: size_t = 1024; |
There was a problem hiding this comment.
Updated to fix this.
| let ptr = mmap( | ||
| std::ptr::null_mut(), | ||
| 1, | ||
| NonZeroUsize::new(1).unwrap(), |
There was a problem hiding this comment.
Do you think it would be worthwhile to bring in the nonzero_ext crate just for these tests?
| NonZeroUsize::new(1).unwrap(), | |
| nonzero_ext::nonzero!(1) |
There was a problem hiding this comment.
At this point no, but if NonZeroUsize sees wider usage it may become worthwhile.
There was a problem hiding this comment.
BTW, I took a look through my other crates and found exactly zero places where nonzero_ext would help. It turns out that Option<NonZeroXXX> is a lot more common than NonZeroXXX.
JonathanWoollett-Light
force-pushed
the
mmap-non-zero-length
branch
from
eb489a9 to
056c8ff
Compare
These changes make sense to me. I think they make sense as separate PRs to keep the changes incremental. |
test/sys/test_mman.rs
Outdated
| let one_k_non_zero = NonZeroUsize::new(ONE_K).unwrap(); | ||
| let ten_one_k = one_k_non_zero | ||
| .checked_mul(NonZeroUsize::new(10).unwrap()) | ||
| .unwrap(); |
There was a problem hiding this comment.
I'd hate to raise the MSRV just for this one line in a test. How about this instead?
| let one_k_non_zero = NonZeroUsize::new(ONE_K).unwrap(); | |
| let ten_one_k = one_k_non_zero | |
| .checked_mul(NonZeroUsize::new(10).unwrap()) | |
| .unwrap(); | |
| let ten_one_k = NonZeroUsize::new(10 * 1024).unwrap(); |
There was a problem hiding this comment.
Fixed
JonathanWoollett-Light
force-pushed
the
mmap-non-zero-length
branch
2 times, most recently
from
9ab07dc to
d034ea5
Compare
|
Merge conflict. |
JonathanWoollett-Light
force-pushed
the
mmap-non-zero-length
branch
from
d034ea5 to
63c5626
Compare
|
bors retry |
2 participants
When calling
mmapthe passed length needs to be greater than zero, else an error is returned:By specifying the argument as
std::num::NonZeroUsizewe eliminate this error case.