nix
nix copied to clipboard
nix-rust
Converting std::process::Child::id() to struct Pid requires a cast
Currently we need something like:
let child = Command::new("ls").arg("-l").spawn().unwrap();
let pid = Pid::from_raw(child.id() as i32);
while id() returns a u32.
fn id(&self) -> u32
This itself poses no risk on Linux, since the hard PID limit is 2^22, but this cast should not be necessary, imho.
Btw. nix::unistd::Pid is not mentioned in the documentation.
i32 should be correct, this looks like a bug with .id() in stdlib. I'd suggest bringing it up there. I'd love to reduce friction with the stdlib, but it doesn't make sense that they'd return a u32 while the underlying pid_t type on POSIX is i32. Maybe Windows only deals with u32s, so they decided to do that.
Regarding the lack of docs, Pid does have docs in the source. It's not been in a released version, so docs.rs is out of date, but you can generate the docs locally like cargo doc --no-deps --open.
Yep, in glibc it's a singed integer: https://www.gnu.org/software/libc/manual/html_node/Process-Identification.html I'll raise that there.
@marmistrz Thanks for raising that with upstream, let's sit on this for a day or so and see where that discussion goes.
What about:
extern crate num;
use num::Integer;
pub fn<T: Integer> from_raw(pid: T) -> Self;
Just like here: https://play.rust-lang.org/?gist=fde06d824756bb0e4a557539561bda77&version=stable
There's already Pid::from_raw(pid_t). So you're suggesting adding the above as well? I think it'd need to be unsafe or return a Result for it to really be feasible.
What about a ChildExt trait implemented for std::process::Child that adds a pid(&self) -> Pid method?
It's a real pity that we need to cast to obtain a Pid instance.
Another option is to define impl From<&Child> for Pid or Pid::from_child(&Child).
The stdlib uses u32 because it is u32 on Windows. On POSIX platforms, even though the stdlib uses u32 , you can still safely cast it because it is guaranteed to be in the scope of [0, i32::MAX]. So Pid::from_raw(Child::id() as i32) would totally work.
Honestly I don't think we should introduce new traits or other helper functions for this safe type cast
