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.

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

EventFd type #1945

Merged
merged 1 commit into from
Dec 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog/1945.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added `EventFd` type.
1 change: 1 addition & 0 deletions changelog/1945.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecated `sys::eventfd::eventfd`.
6 changes: 3 additions & 3 deletions src/sys/epoll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl EpollEvent {

/// A safe wrapper around [`epoll`](https://man7.org/linux/man-pages/man7/epoll.7.html).
/// ```
/// # use nix::sys::{epoll::{EpollTimeout, Epoll, EpollEvent, EpollFlags, EpollCreateFlags}, eventfd::{eventfd, EfdFlags}};
/// # use nix::sys::{epoll::{EpollTimeout, Epoll, EpollEvent, EpollFlags, EpollCreateFlags}, eventfd::{EventFd, EfdFlags}};
/// # use nix::unistd::write;
/// # use std::os::unix::io::{OwnedFd, FromRawFd, AsFd};
/// # use std::time::{Instant, Duration};
Expand All @@ -84,11 +84,11 @@ impl EpollEvent {
/// let epoll = Epoll::new(EpollCreateFlags::empty())?;
///
/// // Create eventfd & Add event
/// let eventfd = eventfd(0, EfdFlags::empty())?;
/// let eventfd = EventFd::new()?;
/// epoll.add(&eventfd, EpollEvent::new(EpollFlags::EPOLLIN,DATA))?;
///
/// // Arm eventfd & Time wait
/// write(&eventfd, &1u64.to_ne_bytes())?;
/// eventfd.arm()?;
/// let now = Instant::now();
///
/// // Wait on event
Expand Down
71 changes: 69 additions & 2 deletions src/sys/eventfd.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::errno::Errno;
use crate::Result;
use std::os::unix::io::{FromRawFd, OwnedFd};
use crate::{Result,unistd};
use std::os::unix::io::{FromRawFd, OwnedFd, AsRawFd, AsFd, RawFd, BorrowedFd};

libc_bitflags! {
pub struct EfdFlags: libc::c_int {
Expand All @@ -10,8 +10,75 @@ libc_bitflags! {
}
}

#[deprecated(since = "0.28.0", note = "Use EventFd::from_value_and_flags() instead")]
pub fn eventfd(initval: libc::c_uint, flags: EfdFlags) -> Result<OwnedFd> {
let res = unsafe { libc::eventfd(initval, flags.bits()) };

Errno::result(res).map(|r| unsafe { OwnedFd::from_raw_fd(r) })
}

#[derive(Debug)]
#[repr(transparent)]
pub struct EventFd(OwnedFd);
impl EventFd {
/// [`EventFd::from_value_and_flags`] with `init_val = 0` and `flags = EfdFlags::empty()`.
pub fn new() -> Result<Self> {
Self::from_value_and_flags(0, EfdFlags::empty())
}
/// Constructs [`EventFd`] with the given `init_val` and `flags`.
///
/// Wrapper around [`libc::eventfd`].
pub fn from_value_and_flags(init_val: u32, flags: EfdFlags) -> Result<Self> {
let res = unsafe { libc::eventfd(init_val, flags.bits()) };
Errno::result(res).map(|r| Self(unsafe { OwnedFd::from_raw_fd(r) }))
}
/// [`EventFd::from_value_and_flags`] with `init_val = 0` and given `flags`.
pub fn from_flags(flags: EfdFlags) -> Result<Self> {
Self::from_value_and_flags(0, flags)
}
/// [`EventFd::from_value_and_flags`] with given `init_val` and `flags = EfdFlags::empty()`.
pub fn from_value(init_val: u32) -> Result<Self> {
Self::from_value_and_flags(init_val, EfdFlags::empty())
}
/// Arms `self`, a following call to `poll`, `select` or `epoll` will return immediately.
///
/// [`EventFd::write`] with `1`.
pub fn arm(&self) -> Result<usize> {
Copy link
Member

Choose a reason for hiding this comment

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

What is the meaning of the return value, here and for defuse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It passes through the return value of unistd::write.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but what does that mean? What does it even mean to write to an eventfd ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is answered by #1945 (comment)

self.write(1)
}
/// Defuses `self`, a following call to `poll`, `select` or `epoll` will block.
///
/// [`EventFd::write`] with `0`.
pub fn defuse(&self) -> Result<usize> {
self.write(0)
}
/// Enqueues `value` triggers.
///
/// The next `value` calls to `poll`, `select` or `epoll` will return immediately.
///
/// [`EventFd::write`] with `value`.
pub fn write(&self, value: u64) -> Result<usize> {
unistd::write(&self.0,&value.to_ne_bytes())
}
// Reads the value from the file descriptor.
pub fn read(&self) -> Result<u64> {
let mut arr = [0; std::mem::size_of::<u64>()];
unistd::read(self.0.as_raw_fd(),&mut arr)?;
Ok(u64::from_ne_bytes(arr))
}
}
impl AsFd for EventFd {
fn as_fd(&self) -> BorrowedFd {
self.0.as_fd()
}
}
impl AsRawFd for EventFd {
Copy link
Member

Choose a reason for hiding this comment

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

Do you need to impl AsRawFd for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so, is there any alternative approach to get the raw file descriptor you would suggest?

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking that we have AsFd, then we can get a BorrowedFd, then we can extract the raw fd by calling .as_raw_fd() on the BorrowedFd, functionally, they have no difference, but this approach may discourage our user from using the raw fd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think its generally more ergonomic for users to have AsRawFd rather than not have it. I could see someone complaining about not having it, but I can't see someone complaining about having it. I don't see any harm in having it.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it has no harm, let's keep it

fn as_raw_fd(&self) -> RawFd {
self.0.as_raw_fd()
}
}
impl From<EventFd> for OwnedFd {
fn from(x: EventFd) -> OwnedFd {
x.0
}
}