[WIP] Move Ptrace{Request, Event, Options} into enums and bitflags. #461
Conversation
|
The The ptrace_request argument in lib seems to be |
|
We could export all the constants and use something like a |
chaosagent
changed the title
| impl PtraceEvent { | ||
| /// Creates a PtraceEvent from the extra bits of a wait status (status >> 16) | ||
| #[inline] | ||
| pub fn from_c_int(event: libc::c_uint) -> Option<PtraceEvent> { |
There was a problem hiding this comment.
for ::nix::sys::signal::Signal we used Result<Signal> as a return type for the corresponding function. I think this is better, because I don't think anyone would intentionally put a wrong c_uint into from_c_int, so the None return type is never expected, thus should probably be an error type.
There was a problem hiding this comment.
Yeah sounds good I'll change that soon
| #[cfg(any(target_os = "linux", target_os = "android"))] | ||
| #[derive(Eq, PartialEq, Clone, Copy, Debug)] | ||
| #[repr(u32)] | ||
| pub enum PtraceEvent { |
There was a problem hiding this comment.
I think we should use #[repr(i32)] and thus consistently the type, which is used by the ffi function.
There was a problem hiding this comment.
We don't actually need the ffi function anymore, as ptrace has been added to the libc crate (https://doc.rust-lang.org/time/libc/fn.ptrace.html). u32 is consistent with libc's use of c_uint, but not with most C compilers' usage of int for enums (its not defined in the C spec afaik).
I feel like the cleanest way to do this is to remove nix's ffi and just wrap libc as it is and leave the rest up to libc upstream. Thoughts?
There was a problem hiding this comment.
In general, we do defer to libc. I just think that the signatures of libc::ptrace und from_c_[u]int should use the same type (either c_int or c_uint), depending on libc::ptrace, because we have no influence on it.
|
Any willingness to push this over the finish line @chaosagent? This came up again because of #614. I'd love to get the |
|
@chaosagent Just wanted to ping you again on this as I'd really like to clean up our ptrace API. Would you be able to clean this up anytime in the near future? |
|
This was closed by #749. |
Addresses #439.
This is a breaking change.
Currently,
PTRACE_EVENTs aren't defined in libc, so they are hardcoded here.