Skip to content

Commit

Permalink
Relax lifetime requirements for FdSet::{insert, remove, contains} (#2136
Browse files Browse the repository at this point in the history 
)

* Relax lifetime requirements for FdSet::{insert, remove, contains}

Fixes #2130

* Take BorrowedFd as the argument for FdSet::{insert, remove, contains}

&AsFd doesn't work because there are 'static types, like std::fs::File,
which implement AsFd.

* fix changelog & remove unused entries

* fix wrong PR number

---------

Co-authored-by: Steve Lau <stevelauc@outlook.com>
  • Loading branch information
@asomers @SteveLauC
asomers and SteveLauC authored Oct 1, 2023
1 parent e5b2df6 commit c4ba877
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 60 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ This project adheres to [Semantic Versioning](https://semver.org/).

([#2137](https://github.com/nix-rust/nix/pull/2137))

- `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))

## [0.27.1] - 2023-08-28

### Fixed
Expand Down
100 changes: 50 additions & 50 deletions src/sys/select.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::convert::TryFrom;
use std::iter::FusedIterator;
use std::mem;
use std::ops::Range;
use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, RawFd};
use std::os::unix::io::{AsRawFd, BorrowedFd, RawFd};
use std::ptr::{null, null_mut};

pub use libc::FD_SETSIZE;
Expand Down Expand Up @@ -41,21 +41,21 @@ impl<'fd> FdSet<'fd> {
}

/// Add a file descriptor to an `FdSet`
pub fn insert<Fd: AsFd>(&mut self, fd: &'fd Fd) {
assert_fd_valid(fd.as_fd().as_raw_fd());
unsafe { libc::FD_SET(fd.as_fd().as_raw_fd(), &mut self.set) };
pub fn insert(&mut self, fd: BorrowedFd<'fd>) {
assert_fd_valid(fd.as_raw_fd());
unsafe { libc::FD_SET(fd.as_raw_fd(), &mut self.set) };
}

/// Remove a file descriptor from an `FdSet`
pub fn remove<Fd: AsFd>(&mut self, fd: &'fd Fd) {
assert_fd_valid(fd.as_fd().as_raw_fd());
unsafe { libc::FD_CLR(fd.as_fd().as_raw_fd(), &mut self.set) };
pub fn remove(&mut self, fd: BorrowedFd<'fd>) {
assert_fd_valid(fd.as_raw_fd());
unsafe { libc::FD_CLR(fd.as_raw_fd(), &mut self.set) };
}

/// Test an `FdSet` for the presence of a certain file descriptor.
pub fn contains<Fd: AsFd>(&self, fd: &'fd Fd) -> bool {
assert_fd_valid(fd.as_fd().as_raw_fd());
unsafe { libc::FD_ISSET(fd.as_fd().as_raw_fd(), &self.set) }
pub fn contains(&self, fd: BorrowedFd<'fd>) -> bool {
assert_fd_valid(fd.as_raw_fd());
unsafe { libc::FD_ISSET(fd.as_raw_fd(), &self.set) }
}

/// Remove all file descriptors from this `FdSet`.
Expand All @@ -77,8 +77,8 @@ impl<'fd> FdSet<'fd> {
/// let fd_four = unsafe {BorrowedFd::borrow_raw(4)};
/// let fd_nine = unsafe {BorrowedFd::borrow_raw(9)};
/// let mut set = FdSet::new();
/// set.insert(&fd_four);
/// set.insert(&fd_nine);
/// set.insert(fd_four);
/// set.insert(fd_nine);
/// assert_eq!(set.highest().map(|borrowed_fd|borrowed_fd.as_raw_fd()), Some(9));
/// ```
///
Expand All @@ -101,8 +101,8 @@ impl<'fd> FdSet<'fd> {
/// let mut set = FdSet::new();
/// let fd_four = unsafe {BorrowedFd::borrow_raw(4)};
/// let fd_nine = unsafe {BorrowedFd::borrow_raw(9)};
/// set.insert(&fd_four);
/// set.insert(&fd_nine);
/// set.insert(fd_four);
/// set.insert(fd_nine);
/// let fds: Vec<RawFd> = set.fds(None).map(|borrowed_fd|borrowed_fd.as_raw_fd()).collect();
/// assert_eq!(fds, vec![4, 9]);
/// ```
Expand Down Expand Up @@ -134,7 +134,7 @@ impl<'a, 'fd> Iterator for Fds<'a, 'fd> {
fn next(&mut self) -> Option<Self::Item> {
for i in &mut self.range {
let borrowed_i = unsafe { BorrowedFd::borrow_raw(i as RawFd) };
if self.set.contains(&borrowed_i) {
if self.set.contains(borrowed_i) {
return Some(borrowed_i);
}
}
Expand All @@ -153,7 +153,7 @@ impl<'a, 'fd> DoubleEndedIterator for Fds<'a, 'fd> {
fn next_back(&mut self) -> Option<BorrowedFd<'fd>> {
while let Some(i) = self.range.next_back() {
let borrowed_i = unsafe { BorrowedFd::borrow_raw(i as RawFd) };
if self.set.contains(&borrowed_i) {
if self.set.contains(borrowed_i) {
return Some(borrowed_i);
}
}
Expand Down Expand Up @@ -323,21 +323,21 @@ mod tests {
use super::*;
use crate::sys::time::{TimeVal, TimeValLike};
use crate::unistd::{pipe, write};
use std::os::unix::io::RawFd;
use std::os::unix::io::{AsFd, RawFd};

#[test]
fn fdset_insert() {
let mut fd_set = FdSet::new();

for i in 0..FD_SETSIZE {
let borrowed_i = unsafe { BorrowedFd::borrow_raw(i as RawFd) };
assert!(!fd_set.contains(&borrowed_i));
assert!(!fd_set.contains(borrowed_i));
}

let fd_seven = unsafe { BorrowedFd::borrow_raw(7) };
fd_set.insert(&fd_seven);
fd_set.insert(fd_seven);

assert!(fd_set.contains(&fd_seven));
assert!(fd_set.contains(fd_seven));
}

#[test]
Expand All @@ -346,16 +346,16 @@ mod tests {

for i in 0..FD_SETSIZE {
let borrowed_i = unsafe { BorrowedFd::borrow_raw(i as RawFd) };
assert!(!fd_set.contains(&borrowed_i));
assert!(!fd_set.contains(borrowed_i));
}

let fd_seven = unsafe { BorrowedFd::borrow_raw(7) };
fd_set.insert(&fd_seven);
fd_set.remove(&fd_seven);
fd_set.insert(fd_seven);
fd_set.remove(fd_seven);

for i in 0..FD_SETSIZE {
let borrowed_i = unsafe { BorrowedFd::borrow_raw(i as RawFd) };
assert!(!fd_set.contains(&borrowed_i));
assert!(!fd_set.contains(borrowed_i));
}
}

Expand All @@ -364,19 +364,19 @@ mod tests {
fn fdset_clear() {
let mut fd_set = FdSet::new();
let fd_one = unsafe { BorrowedFd::borrow_raw(1) };
let fd_FD_SETSIZE_devided_by_two =
let fd_FD_SETSIZE_divided_by_two =
unsafe { BorrowedFd::borrow_raw((FD_SETSIZE / 2) as RawFd) };
let fd_FD_SETSIZE_minus_one =
unsafe { BorrowedFd::borrow_raw((FD_SETSIZE - 1) as RawFd) };
fd_set.insert(&fd_one);
fd_set.insert(&fd_FD_SETSIZE_devided_by_two);
fd_set.insert(&fd_FD_SETSIZE_minus_one);
fd_set.insert(fd_one);
fd_set.insert(fd_FD_SETSIZE_divided_by_two);
fd_set.insert(fd_FD_SETSIZE_minus_one);

fd_set.clear();

for i in 0..FD_SETSIZE {
let borrowed_i = unsafe { BorrowedFd::borrow_raw(i as RawFd) };
assert!(!fd_set.contains(&borrowed_i));
assert!(!fd_set.contains(borrowed_i));
}
}

Expand All @@ -389,22 +389,22 @@ mod tests {
);
let fd_zero = unsafe { BorrowedFd::borrow_raw(0) };
let fd_ninety = unsafe { BorrowedFd::borrow_raw(90) };
set.insert(&fd_zero);
set.insert(fd_zero);
assert_eq!(
set.highest().map(|borrowed_fd| borrowed_fd.as_raw_fd()),
Some(0)
);
set.insert(&fd_ninety);
set.insert(fd_ninety);
assert_eq!(
set.highest().map(|borrowed_fd| borrowed_fd.as_raw_fd()),
Some(90)
);
set.remove(&fd_zero);
set.remove(fd_zero);
assert_eq!(
set.highest().map(|borrowed_fd| borrowed_fd.as_raw_fd()),
Some(90)
);
set.remove(&fd_ninety);
set.remove(fd_ninety);
assert_eq!(
set.highest().map(|borrowed_fd| borrowed_fd.as_raw_fd()),
None
Expand All @@ -413,9 +413,9 @@ mod tests {
let fd_four = unsafe { BorrowedFd::borrow_raw(4) };
let fd_five = unsafe { BorrowedFd::borrow_raw(5) };
let fd_seven = unsafe { BorrowedFd::borrow_raw(7) };
set.insert(&fd_four);
set.insert(&fd_five);
set.insert(&fd_seven);
set.insert(fd_four);
set.insert(fd_five);
set.insert(fd_seven);
assert_eq!(
set.highest().map(|borrowed_fd| borrowed_fd.as_raw_fd()),
Some(7)
Expand All @@ -433,14 +433,14 @@ mod tests {
.collect::<Vec<_>>(),
vec![]
);
set.insert(&fd_zero);
set.insert(fd_zero);
assert_eq!(
set.fds(None)
.map(|borrowed_fd| borrowed_fd.as_raw_fd())
.collect::<Vec<_>>(),
vec![0]
);
set.insert(&fd_ninety);
set.insert(fd_ninety);
assert_eq!(
set.fds(None)
.map(|borrowed_fd| borrowed_fd.as_raw_fd())
Expand Down Expand Up @@ -470,16 +470,16 @@ mod tests {

write(&w1, b"hi!").unwrap();
let mut fd_set = FdSet::new();
fd_set.insert(&r1);
fd_set.insert(&r2);
fd_set.insert(r1.as_fd());
fd_set.insert(r2.as_fd());

let mut timeout = TimeVal::seconds(10);
assert_eq!(
1,
select(None, &mut fd_set, None, None, &mut timeout).unwrap()
);
assert!(fd_set.contains(&r1));
assert!(!fd_set.contains(&r2));
assert!(fd_set.contains(r1.as_fd()));
assert!(!fd_set.contains(r2.as_fd()));
}

#[test]
Expand All @@ -489,8 +489,8 @@ mod tests {

write(&w1, b"hi!").unwrap();
let mut fd_set = FdSet::new();
fd_set.insert(&r1);
fd_set.insert(&r2);
fd_set.insert(r1.as_fd());
fd_set.insert(r2.as_fd());

let mut timeout = TimeVal::seconds(10);
{
Expand All @@ -512,8 +512,8 @@ mod tests {
.unwrap()
);
}
assert!(fd_set.contains(&r1));
assert!(!fd_set.contains(&r2));
assert!(fd_set.contains(r1.as_fd()));
assert!(!fd_set.contains(r2.as_fd()));
}

#[test]
Expand All @@ -522,8 +522,8 @@ mod tests {
write(&w1, b"hi!").unwrap();
let (r2, _w2) = pipe().unwrap();
let mut fd_set = FdSet::new();
fd_set.insert(&r1);
fd_set.insert(&r2);
fd_set.insert(r1.as_fd());
fd_set.insert(r2.as_fd());

let mut timeout = TimeVal::seconds(10);
assert_eq!(
Expand All @@ -537,7 +537,7 @@ mod tests {
)
.unwrap()
);
assert!(fd_set.contains(&r1));
assert!(!fd_set.contains(&r2));
assert!(fd_set.contains(r1.as_fd()));
assert!(!fd_set.contains(r2.as_fd()));
}
}
20 changes: 10 additions & 10 deletions test/sys/test_select.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use nix::sys::select::*;
use nix::sys::signal::SigSet;
use nix::sys::time::{TimeSpec, TimeValLike};
use nix::unistd::{pipe, write};
use std::os::unix::io::{AsRawFd, BorrowedFd};
use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd};

#[test]
pub fn test_pselect() {
Expand All @@ -13,17 +13,17 @@ pub fn test_pselect() {
let (r2, _w2) = pipe().unwrap();

let mut fd_set = FdSet::new();
fd_set.insert(&r1);
fd_set.insert(&r2);
fd_set.insert(r1.as_fd());
fd_set.insert(r2.as_fd());

let timeout = TimeSpec::seconds(10);
let sigmask = SigSet::empty();
assert_eq!(
1,
pselect(None, &mut fd_set, None, None, &timeout, &sigmask).unwrap()
);
assert!(fd_set.contains(&r1));
assert!(!fd_set.contains(&r2));
assert!(fd_set.contains(r1.as_fd()));
assert!(!fd_set.contains(r2.as_fd()));
}

#[test]
Expand All @@ -33,8 +33,8 @@ pub fn test_pselect_nfds2() {
let (r2, _w2) = pipe().unwrap();

let mut fd_set = FdSet::new();
fd_set.insert(&r1);
fd_set.insert(&r2);
fd_set.insert(r1.as_fd());
fd_set.insert(r2.as_fd());

let timeout = TimeSpec::seconds(10);
assert_eq!(
Expand All @@ -49,8 +49,8 @@ pub fn test_pselect_nfds2() {
)
.unwrap()
);
assert!(fd_set.contains(&r1));
assert!(!fd_set.contains(&r2));
assert!(fd_set.contains(r1.as_fd()));
assert!(!fd_set.contains(r2.as_fd()));
}

macro_rules! generate_fdset_bad_fd_tests {
Expand All @@ -60,7 +60,7 @@ macro_rules! generate_fdset_bad_fd_tests {
#[should_panic]
fn $method() {
let bad_fd = unsafe{BorrowedFd::borrow_raw($fd)};
FdSet::new().$method(&bad_fd);
FdSet::new().$method(bad_fd);
}
)*
}
Expand Down

0 comments on commit c4ba877

Please sign in to comment.