Skip to content

Commit

Permalink
refactor: I/O safety for unistd.rs (nix-rust#2440)
Browse files Browse the repository at this point in the history
* refactor: I/O safety for sys/unistd.rs

* chore: changelog entry

* remove link to dup3_raw() since it is not available on all OSes

* fix: imports

* fix test

* fix test on FreeBSD & update docs
  • Loading branch information
SteveLauC authored Jun 10, 2024
1 parent c86548e commit 458013d
Show file tree
Hide file tree
Showing 17 changed files with 610 additions and 293 deletions.
1 change: 1 addition & 0 deletions changelog/2440.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Module unistd now adopts I/O safety.
10 changes: 2 additions & 8 deletions src/fcntl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ use std::ffi::CStr;
use std::ffi::OsString;
#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
use std::ops::{Deref, DerefMut};
#[cfg(not(target_os = "redox"))]
use std::os::raw;
use std::os::unix::ffi::OsStringExt;
#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
use std::os::unix::io::OwnedFd;
Expand Down Expand Up @@ -234,12 +232,8 @@ libc_bitflags!(
);

/// Computes the raw fd consumed by a function of the form `*at`.
#[cfg(any(
all(feature = "fs", not(target_os = "redox")),
all(feature = "process", linux_android),
all(feature = "fanotify", target_os = "linux")
))]
pub(crate) fn at_rawfd(fd: Option<RawFd>) -> raw::c_int {
#[cfg(all(feature = "fanotify", target_os = "linux"))]
pub(crate) fn at_rawfd(fd: Option<RawFd>) -> RawFd {
fd.unwrap_or(libc::AT_FDCWD)
}

Expand Down
2 changes: 1 addition & 1 deletion src/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl<'fd> PollFd<'fd> {
/// let mut fds = [pfd];
/// poll(&mut fds, PollTimeout::NONE).unwrap();
/// let mut buf = [0u8; 80];
/// read(r.as_raw_fd(), &mut buf[..]);
/// read(&r, &mut buf[..]);
/// ```
// Unlike I/O functions, constructors like this must take `BorrowedFd`
// instead of AsFd or &AsFd. Otherwise, an `OwnedFd` argument would be
Expand Down
4 changes: 2 additions & 2 deletions src/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl IntoRawFd for PtyMaster {

impl io::Read for PtyMaster {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
unistd::read(self.0.as_raw_fd(), buf).map_err(io::Error::from)
unistd::read(&self.0, buf).map_err(io::Error::from)
}
}

Expand All @@ -88,7 +88,7 @@ impl io::Write for PtyMaster {

impl io::Read for &PtyMaster {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
unistd::read(self.0.as_raw_fd(), buf).map_err(io::Error::from)
unistd::read(&self.0, buf).map_err(io::Error::from)
}
}

Expand Down
30 changes: 18 additions & 12 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,unistd};
use std::os::unix::io::{FromRawFd, OwnedFd, AsRawFd, AsFd, RawFd, BorrowedFd};
use crate::{unistd, Result};
use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd, RawFd};

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

#[deprecated(since = "0.28.0", note = "Use EventFd::from_value_and_flags() instead")]
#[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()) };

Expand All @@ -26,9 +29,12 @@ impl EventFd {
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> {
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) }))
}
Expand All @@ -41,29 +47,29 @@ impl EventFd {
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> {
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())
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)?;
unistd::read(&self.0, &mut arr)?;
Ok(u64::from_ne_bytes(arr))
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/sys/fanotify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,11 @@ impl Drop for FanotifyEvent {
if self.0.fd == libc::FAN_NOFD {
return;
}
let e = close(self.0.fd);
// SAFETY:
//
// If this fd is not `FAN_NOFD`, then it should be a valid, owned file
// descriptor, which means we can safely close it.
let e = unsafe { close(self.0.fd) };
if !std::thread::panicking() && e == Err(Errno::EBADF) {
panic!("Closing an invalid file descriptor!");
};
Expand Down Expand Up @@ -362,7 +366,7 @@ impl Fanotify {
let mut events = Vec::new();
let mut offset = 0;

let nread = read(self.fd.as_raw_fd(), &mut buffer)?;
let nread = read(&self.fd, &mut buffer)?;

while (nread - offset) >= metadata_size {
let metadata = unsafe {
Expand Down
2 changes: 1 addition & 1 deletion src/sys/inotify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl Inotify {
let mut events = Vec::new();
let mut offset = 0;

let nread = read(self.fd.as_raw_fd(), &mut buffer)?;
let nread = read(&self.fd, &mut buffer)?;

while (nread - offset) >= header_size {
let event = unsafe {
Expand Down
2 changes: 1 addition & 1 deletion src/sys/timerfd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ impl TimerFd {
///
/// Note: If the alarm is unset, then you will wait forever.
pub fn wait(&self) -> Result<()> {
while let Err(e) = read(self.fd.as_fd().as_raw_fd(), &mut [0u8; 8]) {
while let Err(e) = read(&self.fd, &mut [0u8; 8]) {
if e == Errno::ECANCELED {
break;
}
Expand Down
Loading

0 comments on commit 458013d

Please sign in to comment.