Skip to content

Commit df5877c

Browse files
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>
2 parents 8ecd7bc + 147ed3f commit df5877c

File tree

2 files changed

+16
-19
lines changed

2 files changed

+16
-19
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
1919
- With I/O-safe type applied in `pty::OpenptyResult` and `pty::ForkptyResult`,
2020
users no longer need to manually close the file descriptors in these types.
2121
([#1921](https://github.com/nix-rust/nix/pull/1921))
22+
- The `fd` argument to `sys::signalfd::signalfd` is now of type `Option<impl AsFd>`.
23+
([#1874](https://github.com/nix-rust/nix/pull/1874))
2224

2325
### Fixed
2426
### Removed

src/sys/signalfd.rs

+14-19
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,11 @@
1717
//! signal handlers.
1818
use crate::errno::Errno;
1919
pub use crate::sys::signal::{self, SigSet};
20-
use crate::unistd;
2120
use crate::Result;
2221
pub use libc::signalfd_siginfo as siginfo;
2322

2423
use std::mem;
25-
use std::os::unix::io::{AsRawFd, RawFd};
24+
use std::os::unix::io::{AsRawFd, RawFd, FromRawFd, OwnedFd, AsFd, BorrowedFd};
2625

2726
libc_bitflags! {
2827
pub struct SfdFlags: libc::c_int {
@@ -31,7 +30,6 @@ libc_bitflags! {
3130
}
3231
}
3332

34-
pub const SIGNALFD_NEW: RawFd = -1;
3533
#[deprecated(since = "0.23.0", note = "use mem::size_of::<siginfo>() instead")]
3634
pub const SIGNALFD_SIGINFO_SIZE: usize = mem::size_of::<siginfo>();
3735

@@ -46,13 +44,14 @@ pub const SIGNALFD_SIGINFO_SIZE: usize = mem::size_of::<siginfo>();
4644
/// signalfd (the default handler will be invoked instead).
4745
///
4846
/// See [the signalfd man page for more information](https://man7.org/linux/man-pages/man2/signalfd.2.html)
49-
pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
47+
pub fn signalfd<F: AsFd>(fd: Option<F>, mask: &SigSet, flags: SfdFlags) -> Result<OwnedFd> {
48+
let raw_fd = fd.map_or(-1, |x|x.as_fd().as_raw_fd());
5049
unsafe {
5150
Errno::result(libc::signalfd(
52-
fd as libc::c_int,
51+
raw_fd,
5352
mask.as_ref(),
5453
flags.bits(),
55-
))
54+
)).map(|raw_fd|FromRawFd::from_raw_fd(raw_fd))
5655
}
5756
}
5857

@@ -82,30 +81,30 @@ pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
8281
/// Err(err) => (), // some error happend
8382
/// }
8483
/// ```
85-
#[derive(Debug, Eq, Hash, PartialEq)]
86-
pub struct SignalFd(RawFd);
84+
#[derive(Debug)]
85+
pub struct SignalFd(OwnedFd);
8786

8887
impl SignalFd {
8988
pub fn new(mask: &SigSet) -> Result<SignalFd> {
9089
Self::with_flags(mask, SfdFlags::empty())
9190
}
9291

9392
pub fn with_flags(mask: &SigSet, flags: SfdFlags) -> Result<SignalFd> {
94-
let fd = signalfd(SIGNALFD_NEW, mask, flags)?;
93+
let fd = signalfd(None::<OwnedFd>, mask, flags)?;
9594

9695
Ok(SignalFd(fd))
9796
}
9897

9998
pub fn set_mask(&mut self, mask: &SigSet) -> Result<()> {
100-
signalfd(self.0, mask, SfdFlags::empty()).map(drop)
99+
signalfd(Some(self.0.as_fd()), mask, SfdFlags::empty()).map(drop)
101100
}
102101

103102
pub fn read_signal(&mut self) -> Result<Option<siginfo>> {
104103
let mut buffer = mem::MaybeUninit::<siginfo>::uninit();
105104

106105
let size = mem::size_of_val(&buffer);
107106
let res = Errno::result(unsafe {
108-
libc::read(self.0, buffer.as_mut_ptr() as *mut libc::c_void, size)
107+
libc::read(self.0.as_raw_fd(), buffer.as_mut_ptr() as *mut libc::c_void, size)
109108
})
110109
.map(|r| r as usize);
111110
match res {
@@ -117,18 +116,14 @@ impl SignalFd {
117116
}
118117
}
119118

120-
impl Drop for SignalFd {
121-
fn drop(&mut self) {
122-
let e = unistd::close(self.0);
123-
if !std::thread::panicking() && e == Err(Errno::EBADF) {
124-
panic!("Closing an invalid file descriptor!");
125-
};
119+
impl AsFd for SignalFd {
120+
fn as_fd(&self) -> BorrowedFd {
121+
self.0.as_fd()
126122
}
127123
}
128-
129124
impl AsRawFd for SignalFd {
130125
fn as_raw_fd(&self) -> RawFd {
131-
self.0
126+
self.0.as_raw_fd()
132127
}
133128
}
134129

0 commit comments

Comments
 (0)