dd: open stdout from file descriptor when possible

Open stdout using its file descriptor so that a `dd skip=N` command in
a subprocess does not close stdout.

For example, before this commit, multiple instances of `dd` writing to
stdout and appearing in a single command line could incorrectly result
in an empty stdout for each instance of `dd` after the first:

    $ printf "abcde\n" | (dd bs=1 skip=1 count=1 && dd bs=1 skip=1)  2> /dev/null
    b

After this commit, each `dd` process writes to the file descriptor
referring to stdout:

    $ printf "abcde\n" | (dd bs=1 skip=1 count=1 && dd bs=1 skip=1)  2> /dev/null
    bde

src/uu/dd/src/dd.rs

@@ -25,9 +25,8 @@ mod numbers;
 
 use std::cmp;
 use std::env;
-use std::ffi::OsString;
 use std::fs::{File, OpenOptions};
-use std::io::{self, Read, Seek, SeekFrom, Stdout, Write};
+use std::io::{self, Read, Seek, Write};
 #[cfg(any(target_os = "linux", target_os = "android"))]
 use std::os::unix::fs::OpenOptionsExt;
 #[cfg(unix)]
@@ -124,6 +123,8 @@ impl Source {
     /// the [`std::fs::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.
+    ///
+    /// See also [`Dest::stdout_as_file`].
     #[cfg(unix)]
     fn stdin_as_file() -> Self {
         let fd = io::stdin().as_raw_fd();
@@ -371,7 +372,12 @@ enum Density {
 /// Data destinations.
 enum Dest {
     /// Output to stdout.
-    Stdout(Stdout),
+    #[cfg(not(unix))]
+    Stdout(std::io::Stdout),
+
+    /// Output to stdout, opened from its file descriptor.
+    #[cfg(unix)]
+    StdoutFile(File),
 
     /// Output to a file.
     ///
@@ -389,9 +395,31 @@ enum Dest {
 }
 
 impl Dest {
+    /// Create a destination to stdout using its raw file descriptor.
+    ///
+    /// This returns an instance of the `Dest::StdoutFile` variant,
+    /// using the raw file descriptor of [`std::io::Stdout`] to create
+    /// the [`std::fs::File`] parameter. You can use this instead of
+    /// `Dest::Stdout` to allow writing to stdout without dropping the
+    /// entire contents of stdout when this process terminates.
+    ///
+    /// See also [`Source::stdin_as_file`].
+    #[cfg(unix)]
+    fn stdout_as_file() -> Self {
+        let fd = io::stdout().as_raw_fd();
+        let f = unsafe { File::from_raw_fd(fd) };
+        Self::StdoutFile(f)
+    }
+
     fn fsync(&mut self) -> io::Result<()> {
         match self {
+            #[cfg(not(unix))]
             Self::Stdout(stdout) => stdout.flush(),
+            #[cfg(unix)]
+            Self::StdoutFile(f) => {
+                f.flush()?;
+                f.sync_all()
+            }
             Self::File(f, _) => {
                 f.flush()?;
                 f.sync_all()
@@ -408,7 +436,13 @@ impl Dest {
 
     fn fdatasync(&mut self) -> io::Result<()> {
         match self {
+            #[cfg(not(unix))]
             Self::Stdout(stdout) => stdout.flush(),
+            #[cfg(unix)]
+            Self::StdoutFile(f) => {
+                f.flush()?;
+                f.sync_data()
+            }
             Self::File(f, _) => {
                 f.flush()?;
                 f.sync_data()
@@ -425,7 +459,10 @@ impl Dest {
 
     fn seek(&mut self, n: u64) -> io::Result<u64> {
         match self {
+            #[cfg(not(unix))]
             Self::Stdout(stdout) => io::copy(&mut io::repeat(0).take(n), stdout),
+            #[cfg(unix)]
+            Self::StdoutFile(f) => io::copy(&mut io::repeat(0).take(n), f),
             Self::File(f, _) => f.seek(io::SeekFrom::Start(n)),
             #[cfg(unix)]
             Self::Fifo(f) => {
@@ -466,8 +503,11 @@ impl Write for Dest {
                 Ok(buf.len())
             }
             Self::File(f, _) => f.write(buf),
+            #[cfg(not(unix))]
             Self::Stdout(stdout) => stdout.write(buf),
             #[cfg(unix)]
+            Self::StdoutFile(f) => f.write(buf),
+            #[cfg(unix)]
             Self::Fifo(f) => f.write(buf),
             #[cfg(unix)]
             Self::Sink => Ok(buf.len()),
@@ -476,7 +516,10 @@ impl Write for Dest {
 
     fn flush(&mut self) -> io::Result<()> {
         match self {
+            #[cfg(not(unix))]
             Self::Stdout(stdout) => stdout.flush(),
+            #[cfg(unix)]
+            Self::StdoutFile(f) => f.flush(),
             Self::File(f, _) => f.flush(),
             #[cfg(unix)]
             Self::Fifo(f) => f.flush(),
@@ -503,7 +546,10 @@ struct Output<'a> {
 impl<'a> Output<'a> {
     /// Instantiate this struct with stdout as a destination.
     fn new_stdout(settings: &'a Settings) -> UResult<Self> {
+        #[cfg(not(unix))]
         let mut dst = Dest::Stdout(io::stdout());
+        #[cfg(unix)]
+        let mut dst = Dest::stdout_as_file();
         dst.seek(settings.seek)
             .map_err_context(|| "write error".to_string())?;
         Ok(Self { dst, settings })
@@ -884,47 +930,6 @@ fn below_count_limit(count: &Option<Num>, rstat: &ReadStat, wstat: &WriteStat) -
     }
 }
 
-/// Canonicalized file name of `/dev/stdout`.
-///
-/// For example, if this process were invoked from the command line as
-/// `dd`, then this function returns the [`OsString`] form of
-/// `"/dev/stdout"`. However, if this process were invoked as `dd >
-/// outfile`, then this function returns the canonicalized path to
-/// `outfile`, something like `"/path/to/outfile"`.
-fn stdout_canonicalized() -> OsString {
-    match Path::new("/dev/stdout").canonicalize() {
-        Ok(p) => p.into_os_string(),
-        Err(_) => OsString::from("/dev/stdout"),
-    }
-}
-
-/// Decide whether stdout is being redirected to a seekable file.
-///
-/// For example, if this process were invoked from the command line as
-///
-/// ```sh
-/// dd if=/dev/zero bs=1 count=10 seek=5 > /dev/sda1
-/// ```
-///
-/// where `/dev/sda1` is a seekable block device then this function
-/// would return true. If invoked as
-///
-/// ```sh
-/// dd if=/dev/zero bs=1 count=10 seek=5
-/// ```
-///
-/// then this function would return false.
-fn is_stdout_redirected_to_seekable_file() -> bool {
-    let s = stdout_canonicalized();
-    let p = Path::new(&s);
-    match File::open(p) {
-        Ok(mut f) => {
-            f.stream_position().is_ok() && f.seek(SeekFrom::End(0)).is_ok() && f.rewind().is_ok()
-        }
-        Err(_) => false,
-    }
-}
-
 /// Decide whether the named file is a named pipe, also known as a FIFO.
 #[cfg(unix)]
 fn is_fifo(filename: &str) -> bool {
@@ -960,9 +965,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         #[cfg(unix)]
         Some(ref outfile) if is_fifo(outfile) => Output::new_fifo(Path::new(&outfile), &settings)?,
         Some(ref outfile) => Output::new_file(Path::new(&outfile), &settings)?,
-        None if is_stdout_redirected_to_seekable_file() => {
-            Output::new_file(Path::new(&stdout_canonicalized()), &settings)?
-        }
         None => Output::new_stdout(&settings)?,
     };
     dd_copy(i, o).map_err_context(|| "IO error".to_string())

tests/by-util/test_dd.rs

@@ -1528,3 +1528,16 @@ fn test_multiple_processes_reading_stdin() {
         .succeeds()
         .stdout_only("def\n");
 }
+
+/// Test for writing part of stdout from each of two child processes.
+#[cfg(all(not(windows), feature = "printf"))]
+#[test]
+fn test_multiple_processes_writing_stdout() {
+    let printf = format!("{TESTS_BINARY} printf 'abcde\n'");
+    let dd_skip = format!("{TESTS_BINARY} dd bs=1 skip=1 count=1");
+    let dd = format!("{TESTS_BINARY} dd bs=1 skip=1");
+    UCommand::new()
+        .arg(format!("{printf} | ( {dd_skip}; {dd} ) 2> /dev/null"))
+        .succeeds()
+        .stdout_only("bde\n");
+}