Skip to content

Commit

Permalink
Relax lifetime requirements for PollFd::new (#2134)
Browse files Browse the repository at this point in the history
* Relax lifetime requirements for PollFd::new

Fixes #2118

* Take BorrowedFd as the argument for PollFd::new

&AsFd didn't work because there are 'static types, like std::fs::File,
which implement AsFd.  The created BorrowedFd type within the
PollFd::new method would have a very brief lifetime, but the PhantomData
would capture the lifetime of the std::fs::File.  Taking BorrowFd<'fd>
argument makes the lifetime explicit.

* fix legacy comment

---------

Co-authored-by: Steve Lau <stevelauc@outlook.com>
  • Loading branch information
asomers and SteveLauC committed Oct 1, 2023
1 parent 489621e commit ea88824
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 16 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ This project adheres to [Semantic Versioning](https://semver.org/).
- Changed `openat()` and `Dir::openat()`, now take optional `dirfd`s
([#2139](https://github.com/nix-rust/nix/pull/2139))

- `PollFd::new` now takes a `BorrowedFd` argument, with relaxed lifetime
requirements relative to the previous version.
([#2134](https://github.com/nix-rust/nix/pull/2134))

- `FdSet::{insert, remove, contains}` now take `BorrowedFd` arguments, and have
relaxed lifetime requirements relative to 0.27.1.
([#2136](https://github.com/nix-rust/nix/pull/2136))
Expand Down
35 changes: 23 additions & 12 deletions src/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,35 @@ pub struct PollFd<'fd> {
impl<'fd> PollFd<'fd> {
/// Creates a new `PollFd` specifying the events of interest
/// for a given file descriptor.
//
// Different from other I/O-safe interfaces, here, we have to take `AsFd`
// by reference to prevent the case where the `fd` is closed but it is
// still in use. For example:
///
/// # Examples
/// ```no_run
/// # use std::os::unix::io::{AsFd, AsRawFd, FromRawFd};
/// # use nix::{
/// # poll::{PollFd, PollFlags, poll},
/// # unistd::{pipe, read}
/// # };
/// let (r, w) = pipe().unwrap();
/// let pfd = PollFd::new(r.as_fd(), PollFlags::POLLIN);
/// let mut fds = [pfd];
/// poll(&mut fds, -1).unwrap();
/// let mut buf = [0u8; 80];
/// read(r.as_raw_fd(), &mut buf[..]);
/// ```
// Unlike I/O functions, constructors like this must take `BorrowedFd`
// instead of AsFd or &AsFd. Otherwise, an `OwnedFd` argument would be
// dropped at the end of the method, leaving the structure referencing a
// closed file descriptor. For example:
//
// ```rust
// let (reader, _) = pipe().unwrap();
//
// // If `PollFd::new()` takes `AsFd` by value, then `reader` will be consumed,
// // but the file descriptor of `reader` will still be in use.
// let pollfd = PollFd::new(reader, flag);
//
// let (r, _) = pipe().unwrap();
// let pollfd = PollFd::new(r, flag); // Drops the OwnedFd
// // Do something with `pollfd`, which uses the CLOSED fd.
// ```
pub fn new<Fd: AsFd>(fd: &'fd Fd, events: PollFlags) -> PollFd<'fd> {
pub fn new(fd: BorrowedFd<'fd>, events: PollFlags) -> PollFd<'fd> {
PollFd {
pollfd: libc::pollfd {
fd: fd.as_fd().as_raw_fd(),
fd: fd.as_raw_fd(),
events: events.bits(),
revents: PollFlags::empty().bits(),
},
Expand Down
8 changes: 4 additions & 4 deletions test/test_poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use nix::{
poll::{poll, PollFd, PollFlags},
unistd::{pipe, write},
};
use std::os::unix::io::BorrowedFd;
use std::os::unix::io::{AsFd, BorrowedFd};

macro_rules! loop_while_eintr {
($poll_expr: expr) => {
Expand All @@ -20,7 +20,7 @@ macro_rules! loop_while_eintr {
#[test]
fn test_poll() {
let (r, w) = pipe().unwrap();
let mut fds = [PollFd::new(&r, PollFlags::POLLIN)];
let mut fds = [PollFd::new(r.as_fd(), PollFlags::POLLIN)];

// Poll an idle pipe. Should timeout
let nfds = loop_while_eintr!(poll(&mut fds, 100));
Expand Down Expand Up @@ -52,7 +52,7 @@ fn test_ppoll() {

let timeout = TimeSpec::milliseconds(1);
let (r, w) = pipe().unwrap();
let mut fds = [PollFd::new(&r, PollFlags::POLLIN)];
let mut fds = [PollFd::new(r.as_fd(), PollFlags::POLLIN)];

// Poll an idle pipe. Should timeout
let sigset = SigSet::empty();
Expand All @@ -71,7 +71,7 @@ fn test_ppoll() {
#[test]
fn test_pollfd_events() {
let fd_zero = unsafe { BorrowedFd::borrow_raw(0) };
let mut pfd = PollFd::new(&fd_zero, PollFlags::POLLIN);
let mut pfd = PollFd::new(fd_zero.as_fd(), PollFlags::POLLIN);
assert_eq!(pfd.events(), PollFlags::POLLIN);
pfd.set_events(PollFlags::POLLOUT);
assert_eq!(pfd.events(), PollFlags::POLLOUT);
Expand Down

0 comments on commit ea88824

Please sign in to comment.