feat: refactor stdio handling

Author: xtqqczze

Cargo.lock

@@ -480,44 +480,29 @@ fn get_input_type(path: &OsString) -> CatResult<InputType> {
 /// Writes handle to stdout with no configuration. This allows a
 /// simple memory copy.
 fn print_fast<R: FdReadable>(handle: &mut InputHandle<R>) -> CatResult<()> {
-    let stdout = io::stdout();
-    #[cfg(any(target_os = "linux", target_os = "android"))]
-    let mut stdout = stdout;
     #[cfg(any(target_os = "linux", target_os = "android"))]
     {
         // If we're on Linux or Android, try to use the splice() system call
         // for faster writing. If it works, we're done.
-        if !splice::write_fast_using_splice(handle, &mut stdout)? {
+        if !splice::write_fast_using_splice(handle, &mut io::stdout())? {
             return Ok(());
         }
     }
     // If we're not on Linux or Android, or the splice() call failed,
     // fall back on slower writing.
-    print_unbuffered(handle, stdout)
+    print_unbuffered(handle)
 }
 
 #[cfg_attr(any(target_os = "linux", target_os = "android"), inline(never))] // splice fast-path does not require this allocation
-fn print_unbuffered<R: FdReadable>(
-    handle: &mut InputHandle<R>,
-    stdout: io::Stdout,
-) -> CatResult<()> {
-    #[cfg(any(unix, target_os = "wasi"))]
-    let mut stdout = uucore::io::RawWriter(stdout); // use raw syscall to remove buffering
-    #[cfg(not(any(unix, target_os = "wasi")))]
-    let mut stdout = stdout.lock();
+fn print_unbuffered<R: FdReadable>(handle: &mut InputHandle<R>) -> CatResult<()> {
+    let mut stdout = uucore::stdio::stdout_raw();
     let mut buf = [0; 1024 * 64];
     loop {
         match handle.reader.read(&mut buf) {
             Ok(0) => return Ok(()),
-            Ok(n) => {
-                stdout
-                    .write_all(&buf[..n])
-                    .inspect_err(handle_broken_pipe)?;
-                // cannot use rustix::io on Windows
-                // really bad workaround for unbuffered write <https://github.com/uutils/coreutils/issues/12188>
-                #[cfg(not(any(unix, target_os = "wasi")))]
-                stdout.flush().inspect_err(handle_broken_pipe)?;
-            }
+            Ok(n) => stdout
+                .write_all(&buf[..n])
+                .inspect_err(handle_broken_pipe)?,
             Err(e) if e.kind() != ErrorKind::Interrupted => return Err(e.into()),
             _ => {}
         }

src/uu/cat/src/cat.rs

@@ -20,17 +20,18 @@ doctest = false
 
 [dependencies]
 clap = { workspace = true }
+fluent = { workspace = true }
 gcd = { workspace = true }
 libc = { workspace = true }
+rustix = { workspace = true }
+thiserror = { workspace = true }
 uucore = { workspace = true, features = [
   "format",
   "parser-size",
   "quoting-style",
   "fs",
   "signals",
 ] }
-thiserror = { workspace = true }
-fluent = { workspace = true }
 
 [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
 nix = { workspace = true, features = ["fs", "signal"] }

src/uu/dd/Cargo.toml

@@ -25,7 +25,7 @@ use progress::ProgUpdateType;
 use progress::{ProgUpdate, ReadStat, StatusLevel, WriteStat, gen_prog_updater};
 #[cfg(target_os = "linux")]
 use progress::{check_and_reset_sigusr1, install_sigusr1_handler};
-use uucore::io::OwnedFileDescriptorOrHandle;
+use uucore::stdio::{StdinRaw, StdoutRaw};
 use uucore::translate;
 
 use std::cmp;
@@ -37,15 +37,12 @@ use std::fs::{File, OpenOptions};
 use std::io::{self, Read, Seek, SeekFrom, Write};
 #[cfg(any(target_os = "linux", target_os = "android"))]
 use std::os::fd::AsFd;
+#[cfg(unix)]
+use std::os::unix::fs::FileTypeExt;
 #[cfg(any(target_os = "linux", target_os = "android"))]
 use std::os::unix::fs::OpenOptionsExt;
-#[cfg(unix)]
-use std::os::unix::{
-    fs::FileTypeExt,
-    io::{AsRawFd, FromRawFd},
-};
 #[cfg(windows)]
-use std::os::windows::{fs::MetadataExt, io::AsHandle};
+use std::os::windows::fs::MetadataExt;
 use std::path::Path;
 use std::sync::atomic::AtomicU8;
 use std::sync::{Arc, atomic::Ordering::Relaxed, mpsc};
@@ -178,20 +175,20 @@ impl Num {
     }
 }
 
-/// Read and discard `n` bytes from `reader` using a buffer of size `buf_size`.
+/// Read and discard `n` bytes from `file` using a buffer of size `buf_size`.
 ///
 /// This is more efficient than `io::copy` with `BufReader` because it reads
 /// directly in `buf_size`-sized chunks, matching GNU dd's behavior.
 /// Returns the total number of bytes actually read.
-fn read_and_discard<R: Read>(reader: &mut R, n: u64, buf_size: usize) -> io::Result<u64> {
+fn read_and_discard(file: &mut File, n: u64, buf_size: usize) -> io::Result<u64> {
     // todo: consider splice()ing to /dev/null on Linux
     let mut buf = Vec::with_capacity(buf_size);
     let mut total = 0u64;
     let mut remaining = n;
     while remaining > 0 {
         let to_read = cmp::min(remaining, buf_size as u64);
         buf.clear();
-        match reader.by_ref().take(to_read).read_to_end(&mut buf) {
+        match file.take(to_read).read_to_end(&mut buf) {
             Ok(0) => break, // EOF
             Ok(bytes_read) => {
                 total += bytes_read as u64;
@@ -207,44 +204,25 @@ fn read_and_discard<R: Read>(reader: &mut R, n: u64, buf_size: usize) -> io::Res
 
 /// Data sources.
 ///
-/// Use [`Source::stdin_as_file`] if available to enable more
+/// Use [`Source::StdinRaw`] if available to enable more
 /// fine-grained access to reading from stdin.
 enum Source {
-    /// Input from stdin.
-    #[cfg(not(unix))]
-    Stdin(io::Stdin),
-
     /// Input from a file.
     File(File),
 
     /// Input from stdin, opened from its file descriptor.
-    #[cfg(unix)]
-    StdinFile(File),
+    StdinRaw(StdinRaw),
 
     /// Input from a named pipe, also known as a FIFO.
     #[cfg(unix)]
     Fifo(File),
 }
 
 impl Source {
-    /// Create a source from stdin using its raw file descriptor.
-    ///
-    /// This returns an instance of the `Source::StdinFile` variant,
-    /// using the raw file descriptor of [`io::Stdin`] to create
-    /// the [`File`] parameter. You can use this instead of
-    /// `Source::Stdin` to allow reading from stdin without consuming
-    /// the entire contents of stdin when this process terminates.
-    #[cfg(unix)]
-    fn stdin_as_file() -> Self {
-        let fd = io::stdin().as_raw_fd();
-        let f = unsafe { File::from_raw_fd(fd) };
-        Self::StdinFile(f)
-    }
-
     fn skip(&mut self, n: u64, ibs: usize) -> io::Result<u64> {
         match self {
             #[cfg(not(unix))]
-            Self::Stdin(stdin) => {
+            Self::StdinRaw(stdin) => {
                 let m = read_and_discard(stdin, n, ibs)?;
                 if m < n {
                     show_error!(
@@ -255,8 +233,8 @@ impl Source {
                 Ok(m)
             }
             #[cfg(unix)]
-            Self::StdinFile(f) => {
-                if let Ok(Some(len)) = try_get_len_of_block_device(f)
+            Self::StdinRaw(stdin) => {
+                if let Ok(Some(len)) = try_get_len_of_block_device(stdin)
                     && len < n
                 {
                     // GNU compatibility:
@@ -269,9 +247,9 @@ impl Source {
                     return Ok(len);
                 }
                 // Get file length before seeking to avoid race condition
-                let file_len = f.metadata().as_ref().map_or(u64::MAX, Metadata::len);
+                let file_len = stdin.metadata().as_ref().map_or(u64::MAX, Metadata::len);
                 // Try seek first; fall back to read if not seekable
-                match n.try_into().ok().map(|n| f.seek(SeekFrom::Current(n))) {
+                match n.try_into().ok().map(|n| stdin.seek(SeekFrom::Current(n))) {
                     Some(Ok(pos)) => {
                         if pos > file_len {
                             show_error!(
@@ -284,7 +262,7 @@ impl Source {
                     // ESPIPE means the file descriptor is not seekable (e.g., a pipe),
                     // so fall back to reading and discarding bytes using ibs-sized buffer
                     Some(Err(e)) if e.raw_os_error() == Some(libc::ESPIPE) => {
-                        let m = read_and_discard(f, n, ibs)?;
+                        let m = read_and_discard(stdin, n, ibs)?;
                         if m < n {
                             show_error!(
                                 "{}",
@@ -331,11 +309,8 @@ impl Source {
 impl Read for Source {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
         match self {
-            #[cfg(not(unix))]
-            Self::Stdin(stdin) => stdin.read(buf),
             Self::File(f) => f.read(buf),
-            #[cfg(unix)]
-            Self::StdinFile(f) => f.read(buf),
+            Self::StdinRaw(f) => f.read(buf),
             #[cfg(unix)]
             Self::Fifo(f) => f.read(buf),
         }
@@ -359,10 +334,11 @@ struct Input<'a> {
 impl<'a> Input<'a> {
     /// Instantiate this struct with stdin as a source.
     fn new_stdin(settings: &'a Settings) -> UResult<Self> {
+        let stdin = uucore::stdio::stdin_raw();
+
         #[cfg(not(unix))]
         let mut src = {
-            let f = File::from(io::stdin().as_handle().try_clone_to_owned()?);
-            let is_file = if let Ok(metadata) = f.metadata() {
+            let is_file = if let Ok(metadata) = stdin.metadata() {
                 // this hack is needed as there is no other way on windows
                 // to differentiate between the case where `seek` works
                 // on a file handle or not. i.e. when the handle is no real
@@ -373,15 +349,15 @@ impl<'a> Input<'a> {
                 false
             };
             if is_file {
-                Source::File(f)
+                Source::File(stdin.try_clone()?)
             } else {
-                Source::Stdin(io::stdin())
+                Source::StdinRaw(stdin)
             }
         };
         #[cfg(unix)]
-        let mut src = Source::stdin_as_file();
+        let mut src = Source::StdinRaw(stdin);
         #[cfg(unix)]
-        if let Source::StdinFile(f) = &src
+        if let Source::StdinRaw(f) = &src
             && settings.iflags.directory
             && !f.metadata()?.is_dir()
         {
@@ -599,7 +575,7 @@ enum Density {
 /// Data destinations.
 enum Dest {
     /// Output to stdout.
-    Stdout(File),
+    StdoutRaw(StdoutRaw),
 
     /// Output to a file.
     ///
@@ -619,7 +595,7 @@ enum Dest {
 impl Dest {
     fn fsync(&mut self) -> io::Result<()> {
         match self {
-            Self::Stdout(stdout) => stdout.flush(),
+            Self::StdoutRaw(stdout) => stdout.flush(),
             Self::File(f, _) => {
                 f.flush()?;
                 f.sync_all()
@@ -636,7 +612,7 @@ impl Dest {
 
     fn fdatasync(&mut self) -> io::Result<()> {
         match self {
-            Self::Stdout(stdout) => stdout.flush(),
+            Self::StdoutRaw(stdout) => stdout.flush(),
             Self::File(f, _) => {
                 f.flush()?;
                 f.sync_data()
@@ -654,7 +630,7 @@ impl Dest {
     #[cfg_attr(not(unix), allow(unused_variables))]
     fn seek(&mut self, n: u64, obs: usize) -> io::Result<u64> {
         match self {
-            Self::Stdout(stdout) => io::copy(&mut io::repeat(0).take(n), stdout),
+            Self::StdoutRaw(stdout) => io::copy(&mut io::repeat(0).take(n), &mut **stdout),
             Self::File(f, _) => {
                 #[cfg(unix)]
                 if let Ok(Some(len)) = try_get_len_of_block_device(f)
@@ -790,7 +766,7 @@ impl Write for Dest {
                     Err(e) => Err(e),
                 }
             }
-            Self::Stdout(stdout) => stdout.write(buf),
+            Self::StdoutRaw(stdout) => stdout.write(buf),
             #[cfg(unix)]
             Self::Fifo(f) => f.write(buf),
             #[cfg(unix)]
@@ -800,7 +776,7 @@ impl Write for Dest {
 
     fn flush(&mut self) -> io::Result<()> {
         match self {
-            Self::Stdout(stdout) => stdout.flush(),
+            Self::StdoutRaw(stdout) => stdout.flush(),
             Self::File(f, _) => f.flush(),
             #[cfg(unix)]
             Self::Fifo(f) => f.flush(),
@@ -827,8 +803,8 @@ struct Output<'a> {
 impl<'a> Output<'a> {
     /// Instantiate this struct with stdout as a destination.
     fn new_stdout(settings: &'a Settings) -> UResult<Self> {
-        let fx = OwnedFileDescriptorOrHandle::from(io::stdout())?;
-        let mut dst = Dest::Stdout(fx.into_file());
+        let stdout = uucore::stdio::stdout_raw();
+        let mut dst = Dest::StdoutRaw(stdout);
         dst.seek(settings.seek, settings.obs)
             .map_err_context(|| translate!("dd-error-write-error"))?;
         Ok(Self { dst, settings })
@@ -888,16 +864,17 @@ impl<'a> Output<'a> {
     /// already opened by the system (stdout) and has a state
     /// (current position) that shall be used.
     fn new_file_from_stdout(settings: &'a Settings) -> UResult<Self> {
-        let fx = OwnedFileDescriptorOrHandle::from(io::stdout())?;
+        let stdout = uucore::stdio::stdout_raw();
         #[cfg(any(target_os = "linux", target_os = "android"))]
         if let Some(libc_flags) = make_linux_oflags(&settings.oflags) {
             nix::fcntl::fcntl(
-                fx.as_raw().as_fd(),
+                stdout.as_fd(),
                 FcntlArg::F_SETFL(OFlag::from_bits_retain(libc_flags)),
             )?;
         }
 
-        Self::prepare_file(fx.into_file(), settings)
+        // TODO: avoid cloning the underlying file handle
+        Self::prepare_file(stdout.try_clone()?, settings)
     }
 
     /// Instantiate this struct with the given named pipe as a destination.

src/uu/dd/src/dd.rs

@@ -36,7 +36,7 @@ jiff = { workspace = true, optional = true, features = [
   "tzdb-concatenated",
 ] }
 rustc-hash = { workspace = true }
-rustix = { workspace = true, features = ["fs", "pipe", "process"] }
+rustix = { workspace = true, features = ["fs", "pipe", "process", "stdio"] }
 time = { workspace = true, optional = true, features = [
   "formatting",
   "local-offset",

src/uucore/Cargo.toml

@@ -32,6 +32,7 @@ pub use crate::mods::locale;
 pub use crate::mods::os;
 pub use crate::mods::panic;
 pub use crate::mods::posix;
+pub use crate::mods::stdio;
 
 // * feature-gated modules
 #[cfg(feature = "backup-control")]

src/uucore/src/lib/lib.rs

@@ -14,3 +14,4 @@ pub mod locale;
 pub mod os;
 pub mod panic;
 pub mod posix;
+pub mod stdio;

src/uucore/src/lib/mods.rs

@@ -42,23 +42,6 @@ impl<T: AsFd> io::Write for RawWriter<T> {
         Ok(())
     }
 }
-
-// io::write_all but no buffering (deprecated?)
-#[inline]
-#[cfg(any(unix, target_os = "wasi"))]
-pub fn write_all_raw(output: impl AsFd, buf: &[u8]) -> io::Result<()> {
-    let mut written = 0;
-    let len = buf.len();
-    while written < len {
-        match rustix::io::write(&output, &buf[written..]) {
-            Ok(n) => written += n,
-            Err(e) if e.kind() != io::ErrorKind::Interrupted => return Err(e.into()),
-            _ => {}
-        }
-    }
-    Ok(())
-}
-
 /// abstraction wrapper for native file handle / file descriptor
 // todo: remove clone introducing additional syscall dependency
 pub struct OwnedFileDescriptorOrHandle {