Merge #1874

1874: signalfd optional file descriptor r=asomers a=JonathanWoollett-Light

[`sys::signalfd::signalfd`](https://docs.rs/nix/latest/nix/sys/signalfd/fn.signalfd.html) currently takes a `RawFd` for its `fd` argument.

Considering from [the documentation](https://man7.org/linux/man-pages/man2/signalfd.2.html):
> If the fd argument is -1, then the call creates a new file
       descriptor and associates the signal set specified in mask with
       that file descriptor.  If fd is not -1, then it must specify a
       valid existing signalfd file descriptor, and mask is used to
       replace the signal set associated with that file descriptor.

We can better pass the argument as `Option<BorrowedFd>` which encodes the optional nature of this parameter in an option rather than the value being -1 (invalid) (`size_of::<Option<BorrowedFd>>() == size_of::<RawFd>() == 4`).

This removes the error case where `fd < -1`.

> EBADF  The fd file descriptor is not a valid file descriptor.

This does however require additional changes to produce a cohesive implementation, notably changing the type within `Signal` from `RawFd` to `ManuallyDrop<OwnedFd>`, this has no functional affect, but illustrates ownership and allows the type to more easily produce `BorrowedFd`s.

To use [`BorrowedFd`](https://doc.rust-lang.org/stable/std/os/unix/io/struct.BorrowedFd.html) requires updating the MSRV to `>= 1.63.0`

Co-authored-by: Jonathan <jonathanwoollettlight@gmail.com>

1 files changed, 14 insertions, 19 deletions

diff --git a/src/sys/signalfd.rs b/src/sys/signalfd.rs
index 095e5908..a53c8611 100644
--- a/src/sys/signalfd.rs
+++ b/src/sys/signalfd.rs
@@ -17,12 +17,11 @@
 //! signal handlers.
 use crate::errno::Errno;
 pub use crate::sys::signal::{self, SigSet};
-use crate::unistd;
 use crate::Result;
 pub use libc::signalfd_siginfo as siginfo;
 
 use std::mem;
-use std::os::unix::io::{AsRawFd, RawFd};
+use std::os::unix::io::{AsRawFd, RawFd, FromRawFd, OwnedFd, AsFd, BorrowedFd};
 
 libc_bitflags! {
     pub struct SfdFlags: libc::c_int {
@@ -31,7 +30,6 @@ libc_bitflags! {
     }
 }
 
-pub const SIGNALFD_NEW: RawFd = -1;
 #[deprecated(since = "0.23.0", note = "use mem::size_of::<siginfo>() instead")]
 pub const SIGNALFD_SIGINFO_SIZE: usize = mem::size_of::<siginfo>();
 
@@ -46,13 +44,14 @@ pub const SIGNALFD_SIGINFO_SIZE: usize = mem::size_of::<siginfo>();
 /// signalfd (the default handler will be invoked instead).
 ///
 /// See [the signalfd man page for more information](https://man7.org/linux/man-pages/man2/signalfd.2.html)
-pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
+pub fn signalfd<F: AsFd>(fd: Option<F>, mask: &SigSet, flags: SfdFlags) -> Result<OwnedFd> {
+    let raw_fd = fd.map_or(-1, |x|x.as_fd().as_raw_fd());
     unsafe {
         Errno::result(libc::signalfd(
-            fd as libc::c_int,
+            raw_fd,
             mask.as_ref(),
             flags.bits(),
-        ))
+        )).map(|raw_fd|FromRawFd::from_raw_fd(raw_fd))
     }
 }
 
@@ -82,8 +81,8 @@ pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
 ///     Err(err) => (), // some error happend
 /// }
 /// ```
-#[derive(Debug, Eq, Hash, PartialEq)]
-pub struct SignalFd(RawFd);
+#[derive(Debug)]
+pub struct SignalFd(OwnedFd);
 
 impl SignalFd {
     pub fn new(mask: &SigSet) -> Result<SignalFd> {
@@ -91,13 +90,13 @@ impl SignalFd {
     }
 
     pub fn with_flags(mask: &SigSet, flags: SfdFlags) -> Result<SignalFd> {
-        let fd = signalfd(SIGNALFD_NEW, mask, flags)?;
+        let fd = signalfd(None::<OwnedFd>, mask, flags)?;
 
         Ok(SignalFd(fd))
     }
 
     pub fn set_mask(&mut self, mask: &SigSet) -> Result<()> {
-        signalfd(self.0, mask, SfdFlags::empty()).map(drop)
+        signalfd(Some(self.0.as_fd()), mask, SfdFlags::empty()).map(drop)
     }
 
     pub fn read_signal(&mut self) -> Result<Option<siginfo>> {
@@ -105,7 +104,7 @@ impl SignalFd {
 
         let size = mem::size_of_val(&buffer);
         let res = Errno::result(unsafe {
-            libc::read(self.0, buffer.as_mut_ptr() as *mut libc::c_void, size)
+            libc::read(self.0.as_raw_fd(), buffer.as_mut_ptr() as *mut libc::c_void, size)
         })
         .map(|r| r as usize);
         match res {
@@ -117,18 +116,14 @@ impl SignalFd {
     }
 }
 
-impl Drop for SignalFd {
-    fn drop(&mut self) {
-        let e = unistd::close(self.0);
-        if !std::thread::panicking() && e == Err(Errno::EBADF) {
-            panic!("Closing an invalid file descriptor!");
-        };
+impl AsFd for SignalFd {
+    fn as_fd(&self) -> BorrowedFd {
+        self.0.as_fd()
     }
 }
-
 impl AsRawFd for SignalFd {
     fn as_raw_fd(&self) -> RawFd {
-        self.0
+        self.0.as_raw_fd()
     }
 }