Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: make Epoll take BorrowedFd #2232

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

refactor: make Epoll take BorrowedFd #2232

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02f11dc
refactor: make Epoll take BorrowedFd
SteveLauC Nov 27, 2023
23e6f4c
update test and changelog
SteveLauC Nov 28, 2023
de313b3
update test func name
SteveLauC Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 34 additions & 14 deletions src/sys/epoll.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ pub use crate::poll_timeout::PollTimeout as EpollTimeout;
use crate::Result;
use libc::{self, c_int};
use std::mem;
use std::os::unix::io::{AsFd, AsRawFd, FromRawFd, OwnedFd, RawFd};
use std::os::fd::{AsFd, BorrowedFd};
use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd, RawFd};

libc_bitflags!(
pub struct EpollFlags: c_int {
Expand Down Expand Up @@ -85,7 +86,7 @@ impl EpollEvent {
///
/// // Create eventfd & Add event
/// let eventfd = eventfd(0, EfdFlags::empty())?;
/// epoll.add(&eventfd, EpollEvent::new(EpollFlags::EPOLLIN,DATA))?;
/// epoll.add(eventfd.as_fd(), EpollEvent::new(EpollFlags::EPOLLIN,DATA))?;
///
/// // Arm eventfd & Time wait
/// write(&eventfd, &1u64.to_ne_bytes())?;
Expand All @@ -102,37 +103,50 @@ impl EpollEvent {
/// # }
/// ```
#[derive(Debug)]
pub struct Epoll(pub OwnedFd);
impl Epoll {
#[repr(transparent)]
pub struct Epoll<'fd> {
epoll_fd: OwnedFd,
_fds: std::marker::PhantomData<*mut &'fd ()>,
Copy link
Member Author

@SteveLauC SteveLauC Nov 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use _fds: std::marker::PhantomData<BorrowedFd<'fd>> here or the following code would compile without any issue:

use nix::sys::epoll::{Epoll, EpollCreateFlags, EpollEvent};
use nix::unistd::pipe;

fn main() {
    let (r, _w) = pipe().unwrap();
    let epoll = Epoll::new(EpollCreateFlags::empty()).unwrap();
    epoll.add(r, EpollEvent::empty()).unwrap();

    drop(r);
    drop(epoll);
}

I haven't figured out the reason, but using an invariant type here would work

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Friendly ping @asomers, do you have any idea about this?

}

impl<'fd> Epoll<'fd> {
/// Creates a new epoll instance and returns a file descriptor referring to that instance.
///
/// [`epoll_create1`](https://man7.org/linux/man-pages/man2/epoll_create1.2.html).
pub fn new(flags: EpollCreateFlags) -> Result<Self> {
let res = unsafe { libc::epoll_create1(flags.bits()) };
let fd = Errno::result(res)?;
let owned_fd = unsafe { OwnedFd::from_raw_fd(fd) };
Ok(Self(owned_fd))

Ok(Self {
epoll_fd: owned_fd,
_fds: std::marker::PhantomData,
})
}
/// Add an entry to the interest list of the epoll file descriptor for
/// specified in events.
///
/// [`epoll_ctl`](https://man7.org/linux/man-pages/man2/epoll_ctl.2.html) with `EPOLL_CTL_ADD`.
pub fn add<Fd: AsFd>(&self, fd: Fd, mut event: EpollEvent) -> Result<()> {
pub fn add(
&self,
fd: BorrowedFd<'fd>,
mut event: EpollEvent,
) -> Result<()> {
self.epoll_ctl(EpollOp::EpollCtlAdd, fd, &mut event)
}
/// Remove (deregister) the target file descriptor `fd` from the interest list.
///
/// [`epoll_ctl`](https://man7.org/linux/man-pages/man2/epoll_ctl.2.html) with `EPOLL_CTL_DEL` .
pub fn delete<Fd: AsFd>(&self, fd: Fd) -> Result<()> {
pub fn delete(&self, fd: BorrowedFd<'fd>) -> Result<()> {
self.epoll_ctl(EpollOp::EpollCtlDel, fd, None)
}
/// Change the settings associated with `fd` in the interest list to the new settings specified
/// in `event`.
///
/// [`epoll_ctl`](https://man7.org/linux/man-pages/man2/epoll_ctl.2.html) with `EPOLL_CTL_MOD`.
pub fn modify<Fd: AsFd>(
pub fn modify(
&self,
fd: Fd,
fd: BorrowedFd<'fd>,
event: &mut EpollEvent,
) -> Result<()> {
self.epoll_ctl(EpollOp::EpollCtlMod, fd, event)
Expand All @@ -148,7 +162,7 @@ impl Epoll {
) -> Result<usize> {
let res = unsafe {
libc::epoll_wait(
self.0.as_raw_fd(),
self.epoll_fd.as_raw_fd(),
events.as_mut_ptr().cast(),
events.len() as c_int,
timeout.into().into(),
Expand All @@ -164,10 +178,10 @@ impl Epoll {
/// When possible prefer [`Epoll::add`], [`Epoll::delete`] and [`Epoll::modify`].
///
/// [`epoll_ctl`](https://man7.org/linux/man-pages/man2/epoll_ctl.2.html)
fn epoll_ctl<'a, Fd: AsFd, T>(
fn epoll_ctl<'a, T>(
&self,
op: EpollOp,
fd: Fd,
fd: BorrowedFd<'fd>,
event: T,
) -> Result<()>
where
Expand All @@ -179,16 +193,22 @@ impl Epoll {
.unwrap_or(std::ptr::null_mut());
unsafe {
Errno::result(libc::epoll_ctl(
self.0.as_raw_fd(),
self.epoll_fd.as_raw_fd(),
op as c_int,
fd.as_fd().as_raw_fd(),
fd.as_raw_fd(),
ptr,
))
.map(drop)
}
}
}

impl AsFd for Epoll<'_> {
fn as_fd(&self) -> BorrowedFd<'_> {
self.epoll_fd.as_fd()
}
}

#[deprecated(since = "0.27.0", note = "Use Epoll::new() instead")]
#[inline]
pub fn epoll_create() -> Result<RawFd> {
Expand Down