treewide: Modernize nix::sys::ptrace usage #2
Conversation
| let child = unsafe { | ||
| self.pre_exec(|| { | ||
| // Opt-in to ptrace. | ||
| match ptrace::traceme() { |
There was a problem hiding this comment.
N.B.: ptrace::traceme() is the replacement for ptrace(PTRACE_TRACEME, ...). It no longer returns an io::Result (or something directly convertible to an io::Result), so I unrolled it a bit here: system errors (i.e., errno results) get propagated via from_raw_os_error, while everything else acts like the waitpid below and becomes an io::ErrorKind::Other.
There was a problem hiding this comment.
You could write this more concisely as ptrace::traceme().map_err(|e| { // the err case you have below }) I'm not hung up on it enough to even use the review suggestion tool to rewrite it for you, so you can fix it in a followup if you're motivated or it can be what it is. :)
| ptrace(PTRACE_TRACEME, 0, ptr::null_mut(), ptr::null_mut())?; | ||
| Ok(()) | ||
| }).spawn()?; | ||
| let child = unsafe { |
There was a problem hiding this comment.
N.B.: This unsafe is required because of a deprecation: before_exec is now deprecated, and its replacement, pre_exec, is correctly marked as unsafe.
|
ping @luser -- any chance of this getting reviewed? Apologies if you're busy or don't have the bandwidth for it right now 🙂 |
There was a problem hiding this comment.
I'm glad you've found this crate useful, thanks for the fixes! Would you like me to push a new release to crates.io or are you just using it as a git dependency anyway? I figure I should release a new version with this, but if you are actively using it and think you might find other issues that need fixing it can wait a bit.
FWIW, I mostly wrote this crate so I could write this little tool: https://github.com/luser/tracetree
| @@ -6,11 +6,19 @@ use std::process::Command; | |||
| #[test] | |||
| fn readme_test() { | |||
| let rustdoc = Path::new("rustdoc").with_extension(EXE_EXTENSION); | |||
| let readme = Path::new(file!()).parent().unwrap().parent().unwrap().join("README.md"); | |||
| let readme = Path::new(file!()) | |||
There was a problem hiding this comment.
Oh, we should remove this whole file and replace it with the doc-comment crate:
https://crates.io/crates/doc-comment (you don't have to do that now, but I just noted this on a PR on another crate of mine so it's in my head).
| let child = unsafe { | ||
| self.pre_exec(|| { | ||
| // Opt-in to ptrace. | ||
| match ptrace::traceme() { |
There was a problem hiding this comment.
You could write this more concisely as ptrace::traceme().map_err(|e| { // the err case you have below }) I'm not hung up on it enough to even use the review suggestion tool to rewrite it for you, so you can fix it in a followup if you're motivated or it can be what it is. :)
|
Thanks for the merge!
I'm currently using it as a |
Hi there! Thanks for making this crate.
I was looking at using it on a research project that I'm working on, and I noticed that it was no longer compiling correctly. I've taken the liberty of bringing it up to date with the latest
nixrelease and giving it a pass throughcargo fmt. I confirmed locally that the tests still pass.The external-facing API hasn't changed at all.
Please let me know if there's anything else I can do!