search

lnicola / sd-notify

Apache License 2.0
45 stars 6 forks source link

Fixes #2

Closed g2p closed 3 years ago

g2p commented 3 years ago

Hello! I wanted to use listen_fds in another crate, and found a few things that could be fixed. This should be a version bump since some of the types have changed from signed to unsigned.

lnicola commented 3 years ago

Thanks for this and sorry for the poor state this library was in :-(.

lnicola commented 3 years ago

Wait, I'm not sure about the u32 fds. They're signed in the POSIX API, right? I'm thinking that picking std::os::unix::io::RawFd might not be a bad idea.

g2p commented 3 years ago

Thanks for the prompt reply!

They are signed because libc functions can use negative values for errors. (Same for pids and errnos in the other commit) On the Rust side, there's no reason they should be negative. Anyway, the parsing should definitely expect something positive, overly large values are handled by the try_into in fd_cloexec, and the fcntl calls ensure that they are all already-valid file descriptors.

lnicola commented 3 years ago

Yeah, good point about the errors in the C API.

Still, I'm inclined to use RawFd since you can pass them directly to e.g. <File as FromRawFd>::from_raw_fd.

g2p commented 3 years ago

Well, currently sd-notify doesn't open them, and that makes sense (they may be files, sockets, pipes…). The return value is just a count.

g2p commented 3 years ago

(or, return an impl Iter<RawFd>? idk, it's a different approach)

lnicola commented 3 years ago

No, I think it's fine. Published to crates.io, thanks again!