From 64c8a5cab953ebfbbf7abf10e94498ce0aed5994 Mon Sep 17 00:00:00 2001 From: Mark Poliakov Date: Thu, 4 Jan 2024 23:58:12 +0200 Subject: [PATCH] alnyan/yggdrasil: better fd handling in Command --- library/std/src/os/yggdrasil/io/mod.rs | 6 +- library/std/src/sys/yggdrasil/pipe.rs | 16 ++--- library/std/src/sys/yggdrasil/process.rs | 90 ++++++++---------------- 3 files changed, 37 insertions(+), 75 deletions(-) diff --git a/library/std/src/os/yggdrasil/io/mod.rs b/library/std/src/os/yggdrasil/io/mod.rs index a60e06ada19..4caa6b30935 100644 --- a/library/std/src/os/yggdrasil/io/mod.rs +++ b/library/std/src/os/yggdrasil/io/mod.rs @@ -55,10 +55,10 @@ pub unsafe fn mount_raw(options: &MountOptions<'_>) -> crate::io::Result<()> { } #[unstable(feature = "yggdrasil_os", issue = "none")] -pub fn create_pipe_pair() -> crate::io::Result<(RawFd, RawFd)> { +pub fn create_pipe_pair() -> crate::io::Result<(OwnedFd, OwnedFd)> { let mut buffer = MaybeUninit::uninit_array(); cvt_io(unsafe { syscall::create_pipe(&mut buffer) })?; - let read = unsafe { buffer[0].assume_init() }; - let write = unsafe { buffer[1].assume_init() }; + let read = unsafe { OwnedFd::from_raw_fd(buffer[0].assume_init()) }; + let write = unsafe { OwnedFd::from_raw_fd(buffer[1].assume_init()) }; Ok((read, write)) } diff --git a/library/std/src/sys/yggdrasil/pipe.rs b/library/std/src/sys/yggdrasil/pipe.rs index afa08c32dfb..d0a2beeb265 100644 --- a/library/std/src/sys/yggdrasil/pipe.rs +++ b/library/std/src/sys/yggdrasil/pipe.rs @@ -1,5 +1,5 @@ use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut}; -use crate::os::yggdrasil::io::{AsRawFd, FromRawFd, RawFd}; +use crate::os::yggdrasil::io::{AsRawFd, FromRawFd, OwnedFd, RawFd}; use crate::sys::cvt_io; use crate::sys::io::FileDesc; @@ -39,23 +39,15 @@ impl AnonPipe { } } -impl FromRawFd for AnonPipe { - unsafe fn from_raw_fd(fd: RawFd) -> Self { - Self(FileDesc::from_raw_fd(fd)) - } -} - impl AsRawFd for AnonPipe { fn as_raw_fd(&self) -> RawFd { self.0.as_raw_fd() } } -impl Drop for AnonPipe { - fn drop(&mut self) { - unsafe { - // yggdrasil_rt::sys::close(self.as_raw_fd()).ok(); - } +impl From for AnonPipe { + fn from(_fd: OwnedFd) -> Self { + todo!() } } diff --git a/library/std/src/sys/yggdrasil/process.rs b/library/std/src/sys/yggdrasil/process.rs index 808e4ec0efa..951c93a601d 100644 --- a/library/std/src/sys/yggdrasil/process.rs +++ b/library/std/src/sys/yggdrasil/process.rs @@ -2,7 +2,7 @@ use crate::ffi::OsStr; use crate::fmt; use crate::io; use crate::num::NonZeroI32; -use crate::os::yggdrasil::io::{AsRawFd, FromRawFd, IntoRawFd}; +use crate::os::yggdrasil::io::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd}; use crate::path::Path; use crate::sys::cvt_io; use crate::sys::fd::FileDesc; @@ -35,11 +35,12 @@ pub struct ChildPipes { pub stderr: ChildStdio, } -#[derive(Debug, PartialEq)] +#[derive(Debug)] pub enum ChildStdio { Inherit, Null, - Fd(RawFd), + RawFd(RawFd), + OwnedFd(OwnedFd), } #[derive(Debug)] @@ -47,7 +48,8 @@ pub enum Stdio { Inherit, Null, MakePipe, - Fd(FileDesc), + RawFd(RawFd), + OwnedFd(FileDesc), } pub struct Process { @@ -79,8 +81,7 @@ impl Process { impl FromRawFd for crate::process::Stdio { #[inline] unsafe fn from_raw_fd(fd: RawFd) -> crate::process::Stdio { - let fd = FileDesc::from_raw_fd(fd); - let io = Stdio::Fd(fd); + let io = Stdio::RawFd(fd); crate::process::Stdio::from_inner(io) } } @@ -94,24 +95,15 @@ impl Stdio { let (read, write) = crate::os::yggdrasil::io::create_pipe_pair()?; if readable { - yggdrasil_rt::debug_trace!( - "--- Make pipes for MakePipe: to_child={:?}, to_parent={:?} ---", - read, - write - ); - let write = unsafe { AnonPipe::from_raw_fd(write) }; - Ok((ChildStdio::Fd(read), Some(write))) + let write = unsafe { AnonPipe::from(write) }; + Ok((ChildStdio::OwnedFd(read), Some(write))) } else { - yggdrasil_rt::debug_trace!( - "--- Make pipes for MakePipe: to_child={:?}, to_parent={:?} ---", - write, - read - ); - let read = unsafe { AnonPipe::from_raw_fd(read) }; - Ok((ChildStdio::Fd(write), Some(read))) + let read = unsafe { AnonPipe::from(read) }; + Ok((ChildStdio::OwnedFd(write), Some(read))) } } - Stdio::Fd(fd) => Ok((ChildStdio::Fd(fd.as_raw_fd()), None)), + Stdio::OwnedFd(fd) => Ok((ChildStdio::RawFd(fd.as_raw_fd()), None)), + Stdio::RawFd(fd) => Ok((ChildStdio::RawFd(*fd), None)), } } } @@ -128,6 +120,17 @@ impl From for Stdio { } } +impl ChildStdio { + fn as_raw_fd_or_default(&self, default: RawFd) -> Option { + match self { + Self::Null => None, + Self::Inherit => Some(default), + Self::OwnedFd(fd) => Some(fd.as_raw_fd()), + Self::RawFd(fd) => Some(*fd), + } + } +} + // Status #[derive(Clone, Copy, PartialEq, Eq)] @@ -240,7 +243,6 @@ impl Command { } pub fn stdin(&mut self, stdin: Stdio) { - yggdrasil_rt::debug_trace!("!!! Override stdin: {:?}", stdin); self.stdin = stdin; } @@ -273,21 +275,12 @@ impl Command { default: Stdio, needs_stdin: bool, ) -> io::Result<(Process, StdioPipes)> { - yggdrasil_rt::debug_trace!("Default: {:?}, needs_stdin = {:?}", default, needs_stdin); - let default_stdin = if needs_stdin { &default } else { &self.stdin }; let stdin = &self.stdin; let stdout = &self.stdout; let stderr = &self.stderr; - yggdrasil_rt::debug_trace!( - "stdin = {:?}, stdout = {:?}, stderr = {:?}", - stdin, - stdout, - stderr - ); - let (their_stdin, our_stdin) = stdin.to_child_stdio(true)?; let (their_stdout, our_stdout) = stdout.to_child_stdio(false)?; let (their_stderr, our_stderr) = stderr.to_child_stdio(false)?; @@ -295,8 +288,6 @@ impl Command { let ours = StdioPipes { stdin: our_stdin, stdout: our_stdout, stderr: our_stderr }; let theirs = ChildPipes { stdin: their_stdin, stdout: their_stdout, stderr: their_stderr }; - yggdrasil_rt::debug_trace!("theirs = {:?}", theirs); - let process = self.do_spawn(&theirs)?; Ok((process, ours)) @@ -309,35 +300,14 @@ impl Command { fn do_spawn(&mut self, pipes: &ChildPipes) -> io::Result { let mut optional = Vec::new(); - if pipes.stdin != ChildStdio::Null { - optional.push(SpawnOption::InheritFile { - source: match pipes.stdin { - ChildStdio::Inherit => RawFd::STDIN, - ChildStdio::Fd(fd) => fd, - ChildStdio::Null => todo!(), - }, - child: RawFd::STDIN, - }); + if let Some(fd) = pipes.stdin.as_raw_fd_or_default(RawFd::STDIN) { + optional.push(SpawnOption::InheritFile { source: fd, child: RawFd::STDIN }); } - if pipes.stdout != ChildStdio::Null { - optional.push(SpawnOption::InheritFile { - source: match pipes.stdout { - ChildStdio::Inherit => RawFd::STDOUT, - ChildStdio::Fd(fd) => fd, - ChildStdio::Null => todo!(), - }, - child: RawFd::STDOUT, - }); + if let Some(fd) = pipes.stdout.as_raw_fd_or_default(RawFd::STDOUT) { + optional.push(SpawnOption::InheritFile { source: fd, child: RawFd::STDOUT }); } - if pipes.stderr != ChildStdio::Null { - optional.push(SpawnOption::InheritFile { - source: match pipes.stderr { - ChildStdio::Inherit => RawFd::STDERR, - ChildStdio::Fd(fd) => fd, - ChildStdio::Null => todo!(), - }, - child: RawFd::STDERR, - }); + if let Some(fd) = pipes.stderr.as_raw_fd_or_default(RawFd::STDERR) { + optional.push(SpawnOption::InheritFile { source: fd, child: RawFd::STDERR }); } if let Some(pgroup) = self.pgroup {