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.

Sign up for GitHub

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

Jump to bottom

Use upstream libc definitions in mman module #731

Merged
merged 1 commit into from
Aug 11, 2017
Merged

Use upstream libc definitions in mman module #731

merged 1 commit into from
Aug 11, 2017

Conversation

Susurrus
Copy link
Contributor

No description provided.

@Susurrus Susurrus changed the title WIP: Use libc for FFI functions and constants Use upstream libc definitions in mman module Aug 11, 2017
@Susurrus Susurrus force-pushed the mman_ffi branch 3 times, most recently from 8031ab0 to 2e332c6 Compare August 11, 2017 03:02
asomers
asomers requested changes Aug 11, 2017
View reviewed changes
src/sys/mman.rs Outdated
MAP_ANON,
/// The mapping is not backed by any file.
#[cfg(any(target_os = "linux", target_os = "android"))]
MAP_ANONYMOUS,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FreeBSD has this one too, though it seems nobody has added it to libc yet.

src/sys/mman.rs Outdated
MAP_PRIVATE,
/// Place the mapping at exactly the address specified in `addr`.
MAP_FIXED,
/// Synonym for `MAP_ANONYMOUS`. Deprecated.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAP_ANON can't very well be deprecated on platforms where its synonym is undefined, can it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably only deprecated on Linux platforms. In fact it appears that on FreeBSD MAP_ANON is the "correct" constant and MAP_ANONYMOUS is just there for compatibility. I'll just remove the deprecation notice.

src/sys/mman.rs Outdated
/// The mapping is not backed by any file.
#[cfg(any(target_os = "linux", target_os = "android"))]
MAP_ANONYMOUS,
/// Put the mapping the first 2GB of the process address space.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/mapping/mapping into/

src/sys/mman.rs Outdated
/// Put the mapping the first 2GB of the process address space.
#[cfg(any(target_os = "linux", target_os = "android"))]
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
MAP_32BIT,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Present on FreeBSD also.

src/sys/mman.rs Outdated
#[cfg(any(target_os = "linux", target_os = "android"))]
MAP_LOCKED,
/// Do not reserve swap space for this mapping.
MAP_NORESERVE,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAP_NORESERVE and MAP_RENAME were dropped in FreeBSD 11. For compatibility, they're still supported in older binaries. But there are no older binaries using these symbols from Nix. So we should drop the symbols too, at least on FreeBSD.

src/sys/mman.rs Outdated
/// Free up a given range of pages and its associated backing store.
#[cfg(any(target_os = "android", target_os = "linux"))]
MADV_REMOVE,
/// Do not make pages in this rage available to the child after a fork(2).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/rage/range/

src/sys/mman.rs Outdated
pub fn madvise (addr: *const c_void, len: size_t, advice: c_int) -> c_int;
pub fn msync (addr: *const c_void, len: size_t, flags: c_int) -> c_int;
libc_bitflags!{
/// Configuration flags for `msync'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/'/`/

Cargo.toml Outdated
@@ -15,7 +15,7 @@ exclude = [
]

[dependencies]
libc = { git = "https://github.com/rust-lang/libc" }
libc = { git = "https://github.com/susurrus/libc", branch = "mman_constants" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to undo this part before merging.

@Susurrus Susurrus force-pushed the mman_ffi branch 4 times, most recently from 15e401b to 5c25f60 Compare August 11, 2017 06:16
@Susurrus
Copy link
Contributor Author

@asomers I think everything should be ready for merge now that libc has all necessary constants.

@asomers
Copy link
Member

asomers commented Aug 11, 2017

bors r+

bors bot added a commit that referenced this pull request Aug 11, 2017
@bors
Merge #728 #731
5a54f9f 
728: Use upstream libc ptrace FFI r=Susurrus

This might fail on Mac because of how libc defines ptrace on that platform, but we'll see!

731: Use upstream libc definitions in mman module r=asomers
@bors
Copy link
Contributor

bors bot commented Aug 11, 2017

Build succeeded

@bors bors bot merged commit 50fdc68 into nix-rust:master Aug 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asomers asomers asomers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Susurrus @asomers