From b9ba9e57e8bb9516b6ac29f62f6f6a1fb48c64d3 Mon Sep 17 00:00:00 2001 From: PolyMeilex Date: Sat, 10 Feb 2024 04:49:07 +0100 Subject: [PATCH] Use nix & Results --- src/builtins/eval.rs | 8 +++--- src/exec.rs | 9 +++---- src/fd_monitor.rs | 2 +- src/fds.rs | 64 ++++++++++++++++++++++++-------------------- src/io.rs | 9 ++++--- 5 files changed, 49 insertions(+), 43 deletions(-) diff --git a/src/builtins/eval.rs b/src/builtins/eval.rs index 73a588dd3..1f995fab3 100644 --- a/src/builtins/eval.rs +++ b/src/builtins/eval.rs @@ -26,11 +26,11 @@ pub fn eval(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt let mut stdout_fill = None; if streams.out_is_piped { match IoBufferfill::create_opts(parser.libdata().pods.read_limit, STDOUT_FILENO) { - None => { + Err(_) => { // We were unable to create a pipe, probably fd exhaustion. return STATUS_CMD_ERROR; } - Some(buffer) => { + Ok(buffer) => { stdout_fill = Some(buffer.clone()); ios.push(buffer); } @@ -41,11 +41,11 @@ pub fn eval(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt let mut stderr_fill = None; if streams.err_is_piped { match IoBufferfill::create_opts(parser.libdata().pods.read_limit, STDERR_FILENO) { - None => { + Err(_) => { // We were unable to create a pipe, probably fd exhaustion. return STATUS_CMD_ERROR; } - Some(buffer) => { + Ok(buffer) => { stderr_fill = Some(buffer.clone()); ios.push(buffer); } diff --git a/src/exec.rs b/src/exec.rs index f0de731f6..b397ba7b6 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -136,7 +136,7 @@ pub fn exec_job(parser: &Parser, job: &Job, block_io: IoChain) -> bool { let mut proc_pipes = PartialPipes::default(); std::mem::swap(&mut proc_pipes.read, &mut pipe_next_read); if !p.is_last_in_job { - let Some(pipes) = make_autoclose_pipes() else { + let Ok(pipes) = make_autoclose_pipes() else { FLOGF!(warning, "%ls", wgettext!(PIPE_ERROR)); perror("pipe"); aborted_pipeline = true; @@ -1048,12 +1048,12 @@ fn exec_block_or_func_process( if piped_output_needs_buffering { // Be careful to handle failure, e.g. too many open fds. match IoBufferfill::create() { - Some(tmp) => { + Ok(tmp) => { // Teach the job about its bufferfill, and add it to our io chain. io_chain.push(tmp.clone()); block_output_bufferfill = Some(tmp); } - None => return Err(()), + Err(_) => return Err(()), } } @@ -1477,8 +1477,7 @@ fn exec_subshell_internal( // IO buffer creation may fail (e.g. if we have too many open files to make a pipe), so this may // be null. - let Some(bufferfill) = - IoBufferfill::create_opts(parser.libdata().pods.read_limit, STDOUT_FILENO) + let Ok(bufferfill) = IoBufferfill::create_opts(parser.libdata().pods.read_limit, STDOUT_FILENO) else { *break_expand = true; return STATUS_CMD_ERROR.unwrap(); diff --git a/src/fd_monitor.rs b/src/fd_monitor.rs index eba24e5c1..5ab7e2111 100644 --- a/src/fd_monitor.rs +++ b/src/fd_monitor.rs @@ -62,7 +62,7 @@ impl FdEventSignaller { #[cfg(not(HAVE_EVENTFD))] { // Implementation using pipes. - let Some(pipes) = make_autoclose_pipes() else { + let Ok(pipes) = make_autoclose_pipes() else { perror("pipe"); exit_without_destructors(1); }; diff --git a/src/fds.rs b/src/fds.rs index a8e976ec1..88b5411c3 100644 --- a/src/fds.rs +++ b/src/fds.rs @@ -6,9 +6,8 @@ use crate::tests::prelude::*; use crate::wchar::prelude::*; use crate::wutil::perror; use errno::{errno, set_errno}; -use libc::{ - c_int, EINTR, FD_CLOEXEC, F_DUPFD_CLOEXEC, F_GETFD, F_GETFL, F_SETFD, F_SETFL, O_NONBLOCK, -}; +use libc::{c_int, EINTR, FD_CLOEXEC, F_GETFD, F_GETFL, F_SETFD, F_SETFL, O_NONBLOCK}; +use nix::fcntl::FcntlArg; use nix::{fcntl::OFlag, unistd}; use std::ffi::CStr; use std::io::{self, Read, Write}; @@ -130,35 +129,39 @@ pub struct AutoClosePipes { /// Construct a pair of connected pipes, set to close-on-exec. /// \return None on fd exhaustion. -pub fn make_autoclose_pipes() -> Option { - let mut pipes: [c_int; 2] = [-1, -1]; - +pub fn make_autoclose_pipes() -> nix::Result { #[allow(unused_mut, unused_assignments)] let mut already_cloexec = false; #[cfg(HAVE_PIPE2)] - { - if unsafe { libc::pipe2(&mut pipes[0], libc::O_CLOEXEC) } < 0 { + let pipes = match nix::unistd::pipe2(OFlag::O_CLOEXEC) { + Ok(pipes) => { + already_cloexec = true; + pipes + } + Err(err) => { FLOG!(warning, PIPE_ERROR); perror("pipe2"); - return None; + return Err(err); } - already_cloexec = true; - } + }; #[cfg(not(HAVE_PIPE2))] - if unsafe { libc::pipe(&mut pipes[0]) } < 0 { - FLOG!(warning, PIPE_ERROR); - perror("pipe2"); - return None; - } + let pipes = match nix::unistd::pipe() { + Ok(pipes) => pipes, + Err(err) => { + FLOG!(warning, PIPE_ERROR); + perror("pipe2"); + return Err(err); + } + }; - let readp = unsafe { OwnedFd::from_raw_fd(pipes[0]) }; - let writep = unsafe { OwnedFd::from_raw_fd(pipes[1]) }; + let readp = unsafe { OwnedFd::from_raw_fd(pipes.0) }; + let writep = unsafe { OwnedFd::from_raw_fd(pipes.1) }; // Ensure our fds are out of the user range. let readp = heightenize_fd(readp, already_cloexec)?; let writep = heightenize_fd(writep, already_cloexec)?; - Some(AutoClosePipes { + Ok(AutoClosePipes { read: readp, write: writep, }) @@ -170,23 +173,26 @@ pub fn make_autoclose_pipes() -> Option { /// setting it again. /// \return the fd, which always has CLOEXEC set; or an invalid fd on failure, in /// which case an error will have been printed, and the input fd closed. -fn heightenize_fd(fd: OwnedFd, input_has_cloexec: bool) -> Option { +fn heightenize_fd(fd: OwnedFd, input_has_cloexec: bool) -> nix::Result { let raw_fd = fd.as_raw_fd(); if raw_fd >= FIRST_HIGH_FD { if !input_has_cloexec { set_cloexec(raw_fd, true); } - return Some(fd); - } - // Here we are asking the kernel to give us a cloexec fd. - let newfd = unsafe { libc::fcntl(raw_fd, F_DUPFD_CLOEXEC, FIRST_HIGH_FD) }; - if newfd < 0 { - perror("fcntl"); - return None; + return Ok(fd); } - Some(unsafe { OwnedFd::from_raw_fd(newfd) }) + // Here we are asking the kernel to give us a cloexec fd. + let newfd = match nix::fcntl::fcntl(raw_fd, FcntlArg::F_DUPFD_CLOEXEC(FIRST_HIGH_FD)) { + Ok(newfd) => newfd, + Err(err) => { + perror("fcntl"); + return Err(err); + } + }; + + Ok(unsafe { OwnedFd::from_raw_fd(newfd) }) } /// Sets CLO_EXEC on a given fd according to the value of \p should_set. @@ -292,7 +298,7 @@ fn test_pipes() { // Note pipe creation may fail due to fd exhaustion; don't fail in that case. let mut pipes = vec![]; for _i in 0..10 { - if let Some(pipe) = make_autoclose_pipes() { + if let Ok(pipe) = make_autoclose_pipes() { pipes.push(pipe); } } diff --git a/src/io.rs b/src/io.rs index 85a62caa7..9e744b673 100644 --- a/src/io.rs +++ b/src/io.rs @@ -21,6 +21,7 @@ use libc::{EAGAIN, EINTR, ENOENT, ENOTDIR, EPIPE, EWOULDBLOCK, STDOUT_FILENO}; use nix::fcntl::OFlag; use nix::sys::stat::Mode; use std::cell::{RefCell, UnsafeCell}; +use std::io; use std::os::fd::{AsRawFd, IntoRawFd, OwnedFd, RawFd}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Condvar, Mutex, MutexGuard}; @@ -345,14 +346,14 @@ pub struct IoBufferfill { impl IoBufferfill { /// Create an io_bufferfill_t which, when written from, fills a buffer with the contents. /// \returns nullptr on failure, e.g. too many open fds. - pub fn create() -> Option> { + pub fn create() -> io::Result> { Self::create_opts(0, STDOUT_FILENO) } /// Create an io_bufferfill_t which, when written from, fills a buffer with the contents. /// \returns nullptr on failure, e.g. too many open fds. /// /// \param target the fd which this will be dup2'd to - typically stdout. - pub fn create_opts(buffer_limit: usize, target: RawFd) -> Option> { + pub fn create_opts(buffer_limit: usize, target: RawFd) -> io::Result> { assert!(target >= 0, "Invalid target fd"); // Construct our pipes. @@ -365,13 +366,13 @@ impl IoBufferfill { Err(e) => { FLOG!(warning, PIPE_ERROR); perror_io("fcntl", &e); - return None; + return Err(e); } } // Our fillthread gets the read end of the pipe; out_pipe gets the write end. let buffer = Arc::new(IoBuffer::new(buffer_limit)); begin_filling(&buffer, pipes.read); - Some(Arc::new(IoBufferfill { + Ok(Arc::new(IoBufferfill { target, write_fd: pipes.write, buffer,