Skip to content

Commit

Permalink
dd: open stdout from file descriptor when possible
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jfinkels authored and sylvestre committed Sep 24, 2023
1 parent e2e42ac commit c1ba7c9
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 2 deletions.
51 changes: 49 additions & 2 deletions src/uu/dd/src/dd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ 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, SeekFrom, Write};
#[cfg(any(target_os = "linux", target_os = "android"))]
use std::os::unix::fs::OpenOptionsExt;
#[cfg(unix)]
Expand Down Expand Up @@ -171,6 +171,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();
Expand Down Expand Up @@ -472,7 +474,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.
///
Expand All @@ -490,9 +497,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) => {

Check warning on line 521 in src/uu/dd/src/dd.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L521

Added line #L521 was not covered by tests
f.flush()?;
f.sync_all()

Check warning on line 523 in src/uu/dd/src/dd.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L523

Added line #L523 was not covered by tests
}
Self::File(f, _) => {
f.flush()?;
f.sync_all()
Expand All @@ -509,7 +538,13 @@ impl Dest {

fn fdatasync(&mut self) -> io::Result<()> {
match self {
#[cfg(not(unix))]
Self::Stdout(stdout) => stdout.flush(),
#[cfg(unix)]
Self::StdoutFile(f) => {

Check warning on line 544 in src/uu/dd/src/dd.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L544

Added line #L544 was not covered by tests
f.flush()?;
f.sync_data()

Check warning on line 546 in src/uu/dd/src/dd.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L546

Added line #L546 was not covered by tests
}
Self::File(f, _) => {
f.flush()?;
f.sync_data()
Expand All @@ -526,7 +561,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) => {
Expand Down Expand Up @@ -594,8 +632,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()),
Expand All @@ -604,7 +645,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(),

Check warning on line 651 in src/uu/dd/src/dd.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L651

Added line #L651 was not covered by tests
Self::File(f, _) => f.flush(),
#[cfg(unix)]
Self::Fifo(f) => f.flush(),
Expand All @@ -631,7 +675,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 })
Expand Down
14 changes: 14 additions & 0 deletions tests/by-util/test_dd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1566,3 +1566,17 @@ fn test_nocache_file() {
.succeeds()
.stderr_only("2048+0 records in\n2048+0 records out\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()

Check failure on line 1579 in tests/by-util/test_dd.rs

View workflow job for this annotation

GitHub Actions / Style and Lint (macos-12, unix)

ERROR: `cargo fmt`: style violation (file:'tests/by-util/test_dd.rs', line:1579; use `cargo fmt -- "tests/by-util/test_dd.rs"`)

Check failure on line 1579 in tests/by-util/test_dd.rs

View workflow job for this annotation

GitHub Actions / Style/format (ubuntu-latest, feat_os_unix)

ERROR: `cargo fmt`: style violation (file:'tests/by-util/test_dd.rs', line:1579; use `cargo fmt -- "tests/by-util/test_dd.rs"`)
.stdout_only("bde\n");
}

0 comments on commit c1ba7c9

Please sign in to comment.