add mlockall and munlockall #876
Conversation
There was a problem hiding this comment.
Please add doccomments and explain why these functions need to be unsafe. Also, the tests are failing on Linux/S390 because the relevant constants are undefined. You'll have to figure out whether Linux does in fact define those constants on S390 and either define them in libc, or conditionalize them in nix.
src/sys/mman.rs
Outdated
| @@ -213,6 +223,14 @@ pub unsafe fn munlock(addr: *const c_void, length: size_t) -> Result<()> { | |||
| Errno::result(libc::munlock(addr, length)).map(drop) | |||
| } | |||
|
|
|||
| pub unsafe fn mlockall(flags: MlockAllFlags) -> Result<()> { | |||
There was a problem hiding this comment.
Why are these functions unsafe?
There was a problem hiding this comment.
You're right, I suppose there's nothing actually unsafe here. Done.
afck
force-pushed
the
master
branch
2 times, most recently
from
ad62086 to
099e0e4
Compare
|
I conditionalized the constants for now. |
|
Could you please check Linux/S390x? I'd rather not needlessly disable obscure platforms simply out of neglect. |
|
I'm not sure how to verify that; it looks like the constants are defined in |
|
libc merged the PR, so you should be able to update this PR as well. No need to update Nix's Cargo.toml. |
|
Thank you, I removed the conditions. |
|
Could you please add some doc comments for the new functions, too? |
|
I also documented |
|
Ok, I think we're done here. bors r+ |
|
👎 Rejected by code reviews |
|
Hm, bors's behavior has changed. It appears that I now need to click "approve changes" before telling bors to merge. bors r+ |
Timed out |
|
Let's retry. bors r+ |
Closes #875