-
Notifications
You must be signed in to change notification settings - Fork 673
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
Use upstream libc ptrace FFI #728
Conversation
We don't expose this on mac, which is good because that API is considerably different. So this should be good to merge. |
@marmistrz Wanna give this a quick look-over since it pertains to the ptrace work you've been doing? |
src/sys/ptrace.rs
Outdated
#[cfg(not(any(all(target_os = "linux", arch = "s390x"), | ||
all(target_os = "linux", target_env = "gnu"))))] | ||
#[doc(hidden)] | ||
pub type ptrace_request_type = ::libc::c_int; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__ptrace_request
is an enum in C. Won't we be cafe with an int?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enums are c_int
by default in C. But I'm following the argument type dictated by libc
's definition of ptrace()
across all supported platforms.
src/sys/ptrace.rs
Outdated
@@ -83,7 +86,7 @@ pub fn ptrace(request: ptrace::PtraceRequest, pid: Pid, addr: *mut c_void, data: | |||
fn ptrace_peek(request: ptrace::PtraceRequest, pid: Pid, addr: *mut c_void, data: *mut c_void) -> Result<c_long> { | |||
let ret = unsafe { | |||
Errno::clear(); | |||
ffi::ptrace(request, pid.into(), addr, data) | |||
libc::ptrace(request, Into::<libc::pid_t>::into(pid), addr, data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libc::pid_t::from(pid)
would be cleaner. same in several other places.
src/sys/ptrace.rs
Outdated
#[cfg(any(all(target_os = "linux", arch = "s390x"), | ||
all(target_os = "linux", target_env = "gnu")))] | ||
#[doc(hidden)] | ||
pub type ptrace_request_type = ::libc::c_uint; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're importing libc::self
, so the leading ::
is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually libc::self
is not imported into this module, so the leading ::
is necessary here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, indeed, it's a submodule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we'll remove it when we refactor all these constants as part of #724.
@marmistrz Thanks for the review. This should clean things up a bit nicer. |
bors r+ marmistrz |
Merge conflict |
bors r+ marmistrz |
Used the libc_enum! macro to create enums for the ptrace event, request, and libc_bitflags for options constants defined in libc. Also, replicated functionality to move from c_int to PtraceEvent enum in PR nix-rust#728 as it appears to be abandoned. Added utility function for detaching from tracee. Updated names and removed ptrace::ptrace namespace
Used the libc_enum! macro to create enums for the ptrace event, request, and libc_bitflags for options constants defined in libc. Also, replicated functionality to move from c_int to PtraceEvent enum in PR nix-rust#728 as it appears to be abandoned. Added utility function for detaching from tracee. Updated names and removed ptrace::ptrace namespace
This might fail on Mac because of how libc defines ptrace on that platform, but we'll see!