FdSet: allow conversion from iterable of AsFd

[rhelmot]

trafficstars

What does this PR do

Adds an implementation of FromIterator and From for FdSet, allowing them to be constructed from iterables of types that implement AsFd.

Checklist:

• [X] I have read CONTRIBUTING.md
• [X] I have written necessary tests and rustdoc comments
• [X] A change log has been added if this PR modifies nix's API

[TheJonny]

I'm not sure what the better type is, but I think the iterator item type should be the same as the argument type ofFdSet::insert, so that the implementation would be just

for fd in iter.into_iter() {
    result.insert(fd)
}

instead of duplicating the unsafe implementation of insert.

would that be ok: https://github.com/nix-rust/nix/commit/7def327db8683ca31f8e684390b21675ac7fd5e5 ?

Or would it be an API breaking change to also change insert?

[rhelmot]

The goal here was definitely to avoid having to add a map in the user's construction code. What if we factored the unsafe code into another function that both insert and this could use?

[TheJonny]

src/poll.rs has a comment why BorrowedFd is used as argument type instead f F: AsFd, which also applies here, I think:

https://github.com/nix-rust/nix/blob/f129095ac6e76505cfa6711cc78e2b5f4430070e/src/poll.rs#L41

AsFd would allow passing an OwnedFd or File, which would be closed on drop, leaving the FdSet with closed fds.

you can reproduce it if you change your test a bit

fn test_fdset_from_iterable() {
    let (r1, w1) = pipe().unwrap();
    let (r2, w2) = pipe().unwrap();
    let reads = [r1, r2]
        .into_iter()
        .map(|fd| (fd.as_raw_fd(), fd))
        .collect::<std::collections::HashMap<_, _>>();
    let writes = [w1, w2];
    let reads_fdset: FdSet = reads.values().into();
    let writes_fdset: FdSet = writes.into();
    for w in writes_fdset.fds(None) {
        nix::unistd::write(w, b"x").expect("should work");
    }
   /// ...
}

panics with EBADF.

[SteveLauC]

The FdSet interfaces indeed have a reason why they are using BorrowedFd rather than Fd: AsFd, see this PR

---

And for that unsafe code snippet,

unsafe { libc::FD_SET(fd.as_raw_fd(), &mut result.set) };

I am not sure I understand what this means,

The goal here was definitely to avoid having to add a map in the user's construction code.

would you like to elaborate on it a bit?

[TheJonny]

The FdSet interfaces indeed have a reason

we might document it :)

[TheJonny]

appart from type issue, I like the idea of the PR: having a way to construct an FdSet from an iterable!

[SteveLauC]

The FdSet interfaces indeed have a reason

we might document it :)

Yeah, I think we can document it in the comments

[rhelmot]

would you like to elaborate on it a bit?

The goal was to avoid this snippet from the linked commit: writes.iter().map(|x| x.as_fd()).into()

I have not forgotten this PR, I am simply sidetracked onto other projects for a bit :)