From 3e2146fd14ed3ba2c1dde9b288f473c06a4bfaa6 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Sat, 23 Sep 2023 10:35:07 -0600 Subject: [PATCH 1/4] Relax lifetime requirements for FdSet::{insert, remove, contains} Fixes #2130 --- CHANGELOG.md | 7 +++++++ src/sys/select.rs | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60604580a0..ab57ca474e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,13 @@ This project adheres to [Semantic Versioning](https://semver.org/). ([#2137](https://github.com/nix-rust/nix/pull/2137)) +## [Unreleased] - ReleaseDate + +### Fixed + +- Relaxed lifetime requirements for `FdSet::{insert, remove, contains}`. + ([#2135](https://github.com/nix-rust/nix/pull/2135)) + ## [0.27.1] - 2023-08-28 ### Fixed diff --git a/src/sys/select.rs b/src/sys/select.rs index 4c5521be90..3b90773984 100644 --- a/src/sys/select.rs +++ b/src/sys/select.rs @@ -41,19 +41,19 @@ impl<'fd> FdSet<'fd> { } /// Add a file descriptor to an `FdSet` - pub fn insert(&mut self, fd: &'fd Fd) { + pub fn insert(&mut self, fd: Fd) { assert_fd_valid(fd.as_fd().as_raw_fd()); unsafe { libc::FD_SET(fd.as_fd().as_raw_fd(), &mut self.set) }; } /// Remove a file descriptor from an `FdSet` - pub fn remove(&mut self, fd: &'fd Fd) { + pub fn remove(&mut self, fd: Fd) { assert_fd_valid(fd.as_fd().as_raw_fd()); unsafe { libc::FD_CLR(fd.as_fd().as_raw_fd(), &mut self.set) }; } /// Test an `FdSet` for the presence of a certain file descriptor. - pub fn contains(&self, fd: &'fd Fd) -> bool { + pub fn contains(&self, fd: Fd) -> bool { assert_fd_valid(fd.as_fd().as_raw_fd()); unsafe { libc::FD_ISSET(fd.as_fd().as_raw_fd(), &self.set) } } From 347d5e3e7938076877b1ea9baeaea5b5a2113b26 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Sat, 30 Sep 2023 09:56:33 -0600 Subject: [PATCH 2/4] 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. --- CHANGELOG.md | 4 +- src/sys/select.rs | 104 ++++++++++++++++++++-------------------- test/sys/test_select.rs | 20 ++++---- 3 files changed, 65 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab57ca474e..8b7aac39d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,8 +25,10 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] - ReleaseDate ### Fixed +### Changed -- Relaxed lifetime requirements for `FdSet::{insert, remove, contains}`. +- `FdSet::{insert, remove, contains}` now take `BorrowedFd` arguments, and have + relaxed lifetime requirements relative to 0.27.1. ([#2135](https://github.com/nix-rust/nix/pull/2135)) ## [0.27.1] - 2023-08-28 diff --git a/src/sys/select.rs b/src/sys/select.rs index 3b90773984..44a409fd2d 100644 --- a/src/sys/select.rs +++ b/src/sys/select.rs @@ -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; @@ -41,21 +41,21 @@ impl<'fd> FdSet<'fd> { } /// Add a file descriptor to an `FdSet` - pub fn insert(&mut self, 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(&mut self, 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(&self, 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`. @@ -72,13 +72,13 @@ impl<'fd> FdSet<'fd> { /// # Example /// /// ``` - /// # use std::os::unix::io::{AsRawFd, BorrowedFd}; + /// # use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd}; /// # use nix::sys::select::FdSet; /// 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)); /// ``` /// @@ -97,12 +97,12 @@ impl<'fd> FdSet<'fd> { /// /// ``` /// # use nix::sys::select::FdSet; - /// # use std::os::unix::io::{AsRawFd, BorrowedFd, RawFd}; + /// # use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, RawFd}; /// 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 = set.fds(None).map(|borrowed_fd|borrowed_fd.as_raw_fd()).collect(); /// assert_eq!(fds, vec![4, 9]); /// ``` @@ -134,7 +134,7 @@ impl<'a, 'fd> Iterator for Fds<'a, 'fd> { fn next(&mut self) -> Option { 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); } } @@ -153,7 +153,7 @@ impl<'a, 'fd> DoubleEndedIterator for Fds<'a, 'fd> { fn next_back(&mut self) -> Option> { 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); } } @@ -323,7 +323,7 @@ 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() { @@ -331,13 +331,13 @@ 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.insert(fd_seven); - assert!(fd_set.contains(&fd_seven)); + assert!(fd_set.contains(fd_seven)); } #[test] @@ -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)); } } @@ -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)); } } @@ -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 @@ -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) @@ -433,14 +433,14 @@ mod tests { .collect::>(), vec![] ); - set.insert(&fd_zero); + set.insert(fd_zero); assert_eq!( set.fds(None) .map(|borrowed_fd| borrowed_fd.as_raw_fd()) .collect::>(), vec![0] ); - set.insert(&fd_ninety); + set.insert(fd_ninety); assert_eq!( set.fds(None) .map(|borrowed_fd| borrowed_fd.as_raw_fd()) @@ -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] @@ -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); { @@ -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] @@ -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!( @@ -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())); } } diff --git a/test/sys/test_select.rs b/test/sys/test_select.rs index 6b4d1c54c1..111e444d49 100644 --- a/test/sys/test_select.rs +++ b/test/sys/test_select.rs @@ -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() { @@ -13,8 +13,8 @@ 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(); @@ -22,8 +22,8 @@ pub fn test_pselect() { 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] @@ -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!( @@ -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 { @@ -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); } )* } From 0ae59cf8ea06193f71d479694a14726eb149badf Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 1 Oct 2023 18:16:28 +0800 Subject: [PATCH 3/4] fix changelog & remove unused entries --- CHANGELOG.md | 5 ----- src/sys/select.rs | 4 ++-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b7aac39d5..5a95d08f24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,11 +22,6 @@ This project adheres to [Semantic Versioning](https://semver.org/). ([#2137](https://github.com/nix-rust/nix/pull/2137)) -## [Unreleased] - ReleaseDate - -### Fixed -### Changed - - `FdSet::{insert, remove, contains}` now take `BorrowedFd` arguments, and have relaxed lifetime requirements relative to 0.27.1. ([#2135](https://github.com/nix-rust/nix/pull/2135)) diff --git a/src/sys/select.rs b/src/sys/select.rs index 44a409fd2d..ea0c41d4be 100644 --- a/src/sys/select.rs +++ b/src/sys/select.rs @@ -72,7 +72,7 @@ impl<'fd> FdSet<'fd> { /// # Example /// /// ``` - /// # use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd}; + /// # use std::os::unix::io::{AsRawFd, BorrowedFd}; /// # use nix::sys::select::FdSet; /// let fd_four = unsafe {BorrowedFd::borrow_raw(4)}; /// let fd_nine = unsafe {BorrowedFd::borrow_raw(9)}; @@ -97,7 +97,7 @@ impl<'fd> FdSet<'fd> { /// /// ``` /// # use nix::sys::select::FdSet; - /// # use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, RawFd}; + /// # use std::os::unix::io::{AsRawFd, BorrowedFd, RawFd}; /// let mut set = FdSet::new(); /// let fd_four = unsafe {BorrowedFd::borrow_raw(4)}; /// let fd_nine = unsafe {BorrowedFd::borrow_raw(9)}; From c0cb5d3ffae0e5159e572832fa8635899981e384 Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 1 Oct 2023 18:21:08 +0800 Subject: [PATCH 4/4] fix wrong PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a95d08f24..51cf8a2f46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). - `FdSet::{insert, remove, contains}` now take `BorrowedFd` arguments, and have relaxed lifetime requirements relative to 0.27.1. - ([#2135](https://github.com/nix-rust/nix/pull/2135)) + ([#2136](https://github.com/nix-rust/nix/pull/2136)) ## [0.27.1] - 2023-08-28