Use libc definitions for wait module #732
Conversation
src/sys/wait.rs
Outdated
|
|
||
| pub fn exited(status: i32) -> bool { | ||
| (status & 0x7F) == 0 | ||
| unsafe { libc::WIFEXITED(status) } |
There was a problem hiding this comment.
Why unsafe? libc::WIFEXITED and friends are not FFI functions.
There was a problem hiding this comment.
They're marked as unsafe in libc actually. They're actually also marked as extern "C" which is also interesting. I definitely don't think they need to be either of those. @alexcrichton do you know why this is?
There was a problem hiding this comment.
Likely just an oversight. We could move them out of the f! block in libc and then they wouldn't be unsafe anymore. However, I wonder if they even belong in libc at all. There aren't many functions in libc that have more than a simple FFI call.
There was a problem hiding this comment.
These are defined as macros or functions by the spec, so it makes sense to keep them in libc even if they're macros on some platforms and need to be implemented.
There was a problem hiding this comment.
Everything in libc is automatically extern and unsafe, matching the underlying implementation if it didn't use macros.
|
So according to POSIX's definition, these functions may be bors r+ |
|
bors r- |
Canceled |
|
Whoops, looking over this again, I think I actually introduced a bug in |
I don't think you did. Linux This PR looks good to me, though I think you could go a bit further and eliminate the |
|
@rocallahan Thanks for checking that, yes it does look like it does the right things on Linux. And I removed the |
|
bors r+ |
Not certain if these are all correct yet, but we can reuse a ton of logic from
libcwithin thewaitmodule.