Skip to content

Commit

Permalink
Move from libc read/write to nix read/write
Browse files Browse the repository at this point in the history
Replace std from_raw_fd/into_raw_fd dance with nix write

Fixup notifyd build
  • Loading branch information
PolyMeilex authored and ridiculousfish committed Jan 21, 2024
1 parent 65cf6ad commit f3e8272
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 119 deletions.
22 changes: 13 additions & 9 deletions src/builtins/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,14 @@ fn read_in_chunks(fd: RawFd, buff: &mut WString, split_null: bool, do_seek: bool

while !finished {
let mut inbuf = [0_u8; READ_CHUNK_SIZE];
let bytes_read = read_blocked(fd, &mut inbuf);

if bytes_read <= 0 {
eof = true;
break;
}
let bytes_read = bytes_read as usize;
let bytes_read = match read_blocked(fd, &mut inbuf) {
Ok(0) | Err(_) => {
eof = true;
break;
}
Ok(read) => read,
};

let bytes_consumed = inbuf[..bytes_read]
.iter()
Expand Down Expand Up @@ -352,9 +353,12 @@ fn read_one_char_at_a_time(

while !finished {
let mut b = [0_u8; 1];
if read_blocked(fd, &mut b) <= 0 {
eof = true;
break;
match read_blocked(fd, &mut b) {
Ok(0) | Err(_) => {
eof = true;
break;
}
_ => {}
}
let b = b[0];

Expand Down
41 changes: 22 additions & 19 deletions src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::wutil::encoding::{mbrtowc, wcrtomb, zero_mbstate, AT_LEAST_MB_LEN_MAX
use crate::wutil::fish_iswalnum;
use bitflags::bitflags;
use core::slice;
use libc::{EINTR, EIO, O_WRONLY, SIGTTOU, SIG_IGN, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
use libc::{EIO, O_WRONLY, SIGTTOU, SIG_IGN, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
use num_traits::ToPrimitive;
use once_cell::sync::OnceCell;
use std::env;
Expand Down Expand Up @@ -1469,10 +1469,10 @@ fn can_be_encoded(wc: char) -> bool {

/// Call read, blocking and repeating on EINTR. Exits on EAGAIN.
/// \return the number of bytes read, or 0 on EOF. On EAGAIN, returns -1 if nothing was read.
pub fn read_blocked(fd: RawFd, buf: &mut [u8]) -> isize {
pub fn read_blocked(fd: RawFd, buf: &mut [u8]) -> nix::Result<usize> {
loop {
let res = unsafe { libc::read(fd, buf.as_mut_ptr().cast(), buf.len()) };
if res < 0 && errno::errno().0 == EINTR {
let res = nix::unistd::read(fd, buf);
if let Err(nix::Error::EINTR) = res {
continue;
}
return res;
Expand All @@ -1496,16 +1496,17 @@ pub fn write_loop<Fd: AsRawFd>(fd: &Fd, buf: &[u8]) -> std::io::Result<usize> {
let fd = fd.as_raw_fd();
let mut total = 0;
while total < buf.len() {
let written =
unsafe { libc::write(fd, buf[total..].as_ptr() as *const _, buf.len() - total) };
if written < 0 {
let errno = errno::errno().0;
if matches!(errno, libc::EAGAIN | libc::EINTR) {
continue;
match nix::unistd::write(fd, &buf[total..]) {
Ok(written) => {
total += written;
}
Err(err) => {
if matches!(err, nix::Error::EAGAIN | nix::Error::EINTR) {
continue;
}
return Err(std::io::Error::from(err));
}
return Err(std::io::Error::from_raw_os_error(errno));
}
total += written as usize;
}
Ok(total)
}
Expand All @@ -1517,15 +1518,17 @@ pub fn write_loop<Fd: AsRawFd>(fd: &Fd, buf: &[u8]) -> std::io::Result<usize> {
pub fn read_loop<Fd: AsRawFd>(fd: &Fd, buf: &mut [u8]) -> std::io::Result<usize> {
let fd = fd.as_raw_fd();
loop {
let read = unsafe { libc::read(fd, buf.as_mut_ptr() as *mut _, buf.len()) };
if read < 0 {
let errno = errno::errno().0;
if matches!(errno, libc::EAGAIN | libc::EINTR) {
continue;
match nix::unistd::read(fd, buf) {
Ok(read) => {
return Ok(read);
}
Err(err) => {
if matches!(err, nix::Error::EAGAIN | nix::Error::EINTR) {
continue;
}
return Err(std::io::Error::from(err));
}
return Err(std::io::Error::from_raw_os_error(errno));
}
return Ok(read as usize);
}
}

Expand Down
25 changes: 13 additions & 12 deletions src/fd_monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,21 @@ impl FdEventSignaller {
let c = 1_u8;
let mut ret;
loop {
ret = unsafe {
libc::write(
self.write_fd(),
&c as *const _ as *const c_void,
std::mem::size_of_val(&c),
)
};
if ret >= 0 || errno().0 != EINTR {
break;
let bytes = c.to_ne_bytes();
ret = nix::unistd::write(self.write_fd(), &bytes);

match ret {
Ok(_) => break,
Err(nix::Error::EINTR) => continue,
Err(_) => break,
}
}
// EAGAIN occurs if either the pipe buffer is full or the eventfd overflows (very unlikely).
if ret < 0 && ![EAGAIN, EWOULDBLOCK].contains(&errno().0) {
perror("write");

if let Err(err) = ret {
// EAGAIN occurs if either the pipe buffer is full or the eventfd overflows (very unlikely).
if ![nix::Error::EAGAIN, nix::Error::EWOULDBLOCK].contains(&err) {
perror("write");
}
}
}

Expand Down
14 changes: 2 additions & 12 deletions src/fds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,13 @@ pub struct AutoCloseFd {

impl Read for AutoCloseFd {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
unsafe {
match libc::read(self.as_raw_fd(), buf.as_mut_ptr() as *mut _, buf.len()) {
-1 => Err(std::io::Error::from_raw_os_error(errno::errno().0)),
bytes => Ok(bytes as usize),
}
}
nix::unistd::read(self.as_raw_fd(), buf).map_err(std::io::Error::from)
}
}

impl Write for AutoCloseFd {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
unsafe {
match libc::write(self.as_raw_fd(), buf.as_ptr() as *const _, buf.len()) {
-1 => Err(std::io::Error::from_raw_os_error(errno::errno().0)),
bytes => Ok(bytes as usize),
}
}
nix::unistd::write(self.as_raw_fd(), buf).map_err(std::io::Error::from)
}

fn flush(&mut self) -> std::io::Result<()> {
Expand Down
4 changes: 1 addition & 3 deletions src/fork_exec/flog_safe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ pub fn flog_impl_async_safe(fd: i32, s: impl FloggableDisplayAsyncSafe) {
let bytes: &[u8] = s.to_flog_str_async_safe(&mut storage);
// Note we deliberately do not retry on signals, etc.
// This is used to report error messages after fork() in the child process.
unsafe {
let _ = libc::write(fd, bytes.as_ptr() as *const libc::c_void, bytes.len());
}
let _ = nix::unistd::write(fd, bytes);
}

/// Variant of FLOG which is async-safe to use after fork().
Expand Down
33 changes: 13 additions & 20 deletions src/history/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ use std::{
time::{Duration, SystemTime, UNIX_EPOCH},
};

use errno::errno;
use libc::{
c_void, lseek, mmap, munmap, EINTR, MAP_ANONYMOUS, MAP_FAILED, MAP_PRIVATE, PROT_READ,
PROT_WRITE, SEEK_END, SEEK_SET,
lseek, mmap, munmap, MAP_ANONYMOUS, MAP_FAILED, MAP_PRIVATE, PROT_READ, PROT_WRITE, SEEK_END,
SEEK_SET,
};

use super::{HistoryItem, PersistenceMode};
Expand Down Expand Up @@ -137,7 +136,7 @@ impl HistoryFileContents {
if unsafe { lseek(fd, 0, SEEK_SET) } != 0 {
return None;
}
if !read_from_fd(fd, region.as_mut()) {
if read_from_fd(fd, region.as_mut()).is_err() {
return None;
}
}
Expand Down Expand Up @@ -230,25 +229,19 @@ fn should_mmap() -> bool {

/// Read from `fd` to fill `dest`, zeroing any unused space.
// Return true on success, false on failure.
fn read_from_fd(fd: RawFd, dest: &mut [u8]) -> bool {
let mut remaining = dest.len();
let mut nread = 0;
while remaining > 0 {
let amt =
unsafe { libc::read(fd, (&mut dest[nread]) as *mut u8 as *mut c_void, remaining) };
if amt < 0 {
if errno().0 != EINTR {
return false;
fn read_from_fd(fd: RawFd, mut dest: &mut [u8]) -> nix::Result<()> {
while !dest.is_empty() {
match nix::unistd::read(fd, dest) {
Ok(0) => break,
Ok(amt) => {
dest = &mut dest[amt..];
}
} else if amt == 0 {
break;
} else {
remaining -= amt as usize;
nread += amt as usize;
Err(nix::Error::EINTR) => continue,
Err(err) => return Err(err),
}
}
dest[nread..].fill(0u8);
true
dest.fill(0u8);
Ok(())
}

fn replace_all(s: &mut Vec<u8>, needle: &[u8], replacement: &[u8]) {
Expand Down
2 changes: 1 addition & 1 deletion src/input_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ fn readb(in_fd: RawFd) -> ReadbResult {
// Check stdin.
if fdset.test(in_fd) {
let mut arr: [u8; 1] = [0];
if read_blocked(in_fd, &mut arr) != 1 {
if read_blocked(in_fd, &mut arr) != Ok(1) {
// The terminal has been closed.
return ReadbResult::Eof;
}
Expand Down
52 changes: 28 additions & 24 deletions src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
//! end of the list is reached, at which point regular searching will commence.

use libc::{
c_char, c_int, c_void, EAGAIN, ECHO, EINTR, EIO, EISDIR, ENOTTY, EPERM, ESRCH, EWOULDBLOCK,
ICANON, ICRNL, IEXTEN, INLCR, IXOFF, IXON, ONLCR, OPOST, O_NONBLOCK, O_RDONLY, SIGINT, SIGTTIN,
STDIN_FILENO, STDOUT_FILENO, S_IFDIR, TCSANOW, VMIN, VQUIT, VSUSP, VTIME, _POSIX_VDISABLE,
c_char, c_int, ECHO, EINTR, EIO, EISDIR, ENOTTY, EPERM, ESRCH, ICANON, ICRNL, IEXTEN, INLCR,
IXOFF, IXON, ONLCR, OPOST, O_NONBLOCK, O_RDONLY, SIGINT, SIGTTIN, STDIN_FILENO, STDOUT_FILENO,
S_IFDIR, TCSANOW, VMIN, VQUIT, VSUSP, VTIME, _POSIX_VDISABLE,
};
use once_cell::sync::Lazy;
use std::cell::UnsafeCell;
Expand Down Expand Up @@ -688,27 +688,31 @@ fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> i32 {
let mut fd_contents = Vec::with_capacity(usize::try_from(buf.st_size).unwrap());
loop {
let mut buff = [0_u8; 4096];
let amt = unsafe { libc::read(fd, &mut buff[0] as *mut _ as *mut c_void, buff.len()) };
if amt > 0 {
fd_contents.extend_from_slice(&buff[..usize::try_from(amt).unwrap()]);
} else if amt == 0 {
// EOF.
break;
} else {
assert!(amt == -1);
let err = errno();
if err.0 == EINTR {
continue;
} else if err.0 == EAGAIN || err.0 == EWOULDBLOCK && make_fd_blocking(fd).is_ok() {
// We succeeded in making the fd blocking, keep going.
continue;
} else {
// Fatal error.
FLOG!(
error,
wgettext_fmt!("Unable to read input file: %s", err.to_string())
);
return 1;

match nix::unistd::read(fd, &mut buff) {
Ok(0) => {
// EOF.
break;
}
Ok(amt) => {
fd_contents.extend_from_slice(&buff[..amt]);
}
Err(err) => {
if err == nix::Error::EINTR {
continue;
} else if err == nix::Error::EAGAIN
|| err == nix::Error::EWOULDBLOCK && make_fd_blocking(fd).is_ok()
{
// We succeeded in making the fd blocking, keep going.
continue;
} else {
// Fatal error.
FLOG!(
error,
wgettext_fmt!("Unable to read input file: %s", err.to_string())
);
return 1;
}
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/tests/fd_monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,8 @@ impl ItemMaker {
}
ItemWakeReason::Readable => {
let mut buf = [0u8; 1024];
let amt =
unsafe { libc::read(fd.as_raw_fd(), buf.as_mut_ptr() as *mut _, buf.len()) };
assert_ne!(amt, -1, "read error!");
let res = nix::unistd::read(fd.as_raw_fd(), &mut buf);
let amt = res.expect("read error!");
self.length_read.fetch_add(amt as usize, Ordering::Relaxed);
was_closed = amt == 0;
}
Expand Down
20 changes: 10 additions & 10 deletions src/universal_notifier/notifyd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,16 @@ impl UniversalNotifier for NotifydNotifier {
let mut read_something = false;
let mut buff: [u8; 64] = [0; 64];
loop {
let amt_read = unsafe {
libc::read(
self.notify_fd,
buff.as_mut_ptr() as *mut libc::c_void,
buff.len(),
)
};
read_something = read_something || amt_read > 0;
if amt_read != buff.len() as isize {
break;
let res = nix::unistd::read(self.notify_fd, &mut buff);

if let Ok(amt_read) = res {
read_something |= amt_read > 0;
}

match res {
Ok(amt_read) if amt_read != buff.len() => break,
Err(_) => break,
_ => continue,
}
}
FLOGF!(
Expand Down
8 changes: 2 additions & 6 deletions src/wutil/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,12 +401,8 @@ pub fn wrename(old_name: &wstr, new_name: &wstr) -> libc::c_int {
unsafe { libc::rename(old_narrow.as_ptr(), new_narrow.as_ptr()) }
}

pub fn write_to_fd(input: &[u8], fd: RawFd) -> std::io::Result<usize> {
let mut file = unsafe { std::fs::File::from_raw_fd(fd) };
let amt = file.write(input);
// Ensure the file is not closed.
file.into_raw_fd();
amt
pub fn write_to_fd(input: &[u8], fd: RawFd) -> nix::Result<usize> {
nix::unistd::write(fd, input)
}

/// Write a wide string to a file descriptor. This avoids doing any additional allocation.
Expand Down

0 comments on commit f3e8272

Please sign in to comment.