Skip to content

Commit

Permalink
re-use existing fd for stdout even if its a seek-able file
Browse files Browse the repository at this point in the history
this is important as the fd holds the file offset we need to use
  • Loading branch information
cre4ture committed Mar 17, 2024
1 parent f8e5296 commit 3de43ad
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 11 deletions.
4 changes: 1 addition & 3 deletions src/uu/dd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@ gcd = { workspace = true }
libc = { workspace = true }
uucore = { workspace = true, features = ["format", "quoting-style"] }

[target.'cfg(any(target_os = "linux"))'.dependencies]
nix = { workspace = true, features = ["fs"] }

[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
signal-hook = { workspace = true }
nix = { workspace = true, features = ["fs"] }

[[bin]]
name = "dd"
Expand Down
59 changes: 52 additions & 7 deletions src/uu/dd/src/dd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.

// spell-checker:ignore fname, ftype, tname, fpath, specfile, testfile, unspec, ifile, ofile, outfile, fullblock, urand, fileio, atoe, atoibm, behaviour, bmax, bremain, cflags, creat, ctable, ctty, datastructures, doesnt, etoa, fileout, fname, gnudd, iconvflags, iseek, nocache, noctty, noerror, nofollow, nolinks, nonblock, oconvflags, oseek, outfile, parseargs, rlen, rmax, rremain, rsofar, rstat, sigusr, wlen, wstat seekable oconv canonicalized fadvise Fadvise FADV DONTNEED ESPIPE bufferedoutput
// spell-checker:ignore fname, ftype, tname, fpath, specfile, testfile, unspec, ifile, ofile, outfile, fullblock, urand, fileio, atoe, atoibm, behaviour, bmax, bremain, cflags, creat, ctable, ctty, datastructures, doesnt, etoa, fileout, fname, gnudd, iconvflags, iseek, nocache, noctty, noerror, nofollow, nolinks, nonblock, oconvflags, oseek, outfile, parseargs, rlen, rmax, rremain, rsofar, rstat, sigusr, wlen, wstat seekable oconv canonicalized fadvise Fadvise FADV DONTNEED ESPIPE bufferedoutput, SETFL

mod blocks;
mod bufferedoutput;
Expand All @@ -16,8 +16,13 @@ mod progress;
use crate::bufferedoutput::BufferedOutput;
use blocks::conv_block_unblock_helper;
use datastructures::*;
#[cfg(any(target_os = "linux", target_os = "android"))]
use nix::fcntl::FcntlArg::F_SETFL;
#[cfg(any(target_os = "linux", target_os = "android"))]
use nix::fcntl::OFlag;
use parseargs::Parser;
use progress::{gen_prog_updater, ProgUpdate, ReadStat, StatusLevel, WriteStat};
use uucore::io::OwnedFileDescriptorOrHandle;

use std::cmp;
use std::env;
Expand All @@ -31,6 +36,8 @@ use std::os::unix::{
fs::FileTypeExt,
io::{AsRawFd, FromRawFd},
};
#[cfg(windows)]
use std::os::windows::{fs::MetadataExt, io::AsHandle};
use std::path::Path;
use std::sync::{
atomic::{AtomicBool, Ordering::Relaxed},
Expand Down Expand Up @@ -227,7 +234,7 @@ impl Source {
Err(e) => Err(e),
}
}
Self::File(f) => f.seek(io::SeekFrom::Start(n)),
Self::File(f) => f.seek(io::SeekFrom::Current(n.try_into().unwrap())),
#[cfg(unix)]
Self::Fifo(f) => io::copy(&mut f.take(n), &mut io::sink()),
}
Expand Down Expand Up @@ -283,7 +290,24 @@ impl<'a> Input<'a> {
/// Instantiate this struct with stdin as a source.
fn new_stdin(settings: &'a Settings) -> UResult<Self> {
#[cfg(not(unix))]
let mut src = Source::Stdin(io::stdin());
let mut src = {
let f = File::from(io::stdin().as_handle().try_clone_to_owned()?);
let is_file = if let Ok(metadata) = f.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
// file but a pipe, `seek` is still successful, but following
// `read`s are not affected by the seek.
metadata.creation_time() != 0
} else {
false
};
if is_file {
Source::File(f)
} else {
Source::Stdin(io::stdin())
}
};
#[cfg(unix)]
let mut src = Source::stdin_as_file();
if settings.skip > 0 {
Expand Down Expand Up @@ -557,7 +581,7 @@ impl Dest {
return Ok(len);
}
}
f.seek(io::SeekFrom::Start(n))
f.seek(io::SeekFrom::Current(n.try_into().unwrap()))
}
#[cfg(unix)]
Self::Fifo(f) => {
Expand Down Expand Up @@ -699,6 +723,11 @@ impl<'a> Output<'a> {
if !settings.oconv.notrunc {
dst.set_len(settings.seek).ok();
}

Self::prepare_file(dst, settings)
}

fn prepare_file(dst: File, settings: &'a Settings) -> UResult<Self> {
let density = if settings.oconv.sparse {
Density::Sparse
} else {
Expand All @@ -710,6 +739,24 @@ impl<'a> Output<'a> {
Ok(Self { dst, settings })
}

/// Instantiate this struct with file descriptor as a destination.
///
/// This is useful e.g. for the case when the file descriptor was
/// 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())?;
#[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_raw_fd(),
F_SETFL(OFlag::from_bits_retain(libc_flags)),
)?;

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

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L754

Added line #L754 was not covered by tests
}

Self::prepare_file(fx.into_file(), settings)
}

/// Instantiate this struct with the given named pipe as a destination.
#[cfg(unix)]
fn new_fifo(filename: &Path, settings: &'a Settings) -> UResult<Self> {
Expand Down Expand Up @@ -1287,9 +1334,7 @@ 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 if is_stdout_redirected_to_seekable_file() => Output::new_file_from_stdout(&settings)?,
None => Output::new_stdout(&settings)?,
};
dd_copy(i, o).map_err_context(|| "IO error".to_string())
Expand Down
1 change: 1 addition & 0 deletions src/uucore/src/lib/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub use uucore_procs::*;
// * cross-platform modules
pub use crate::mods::display;
pub use crate::mods::error;
pub use crate::mods::io;
pub use crate::mods::line_ending;
pub use crate::mods::os;
pub use crate::mods::panic;
Expand Down
1 change: 1 addition & 0 deletions src/uucore/src/lib/mods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

pub mod display;
pub mod error;
pub mod io;
pub mod line_ending;
pub mod os;
pub mod panic;
Expand Down
83 changes: 83 additions & 0 deletions src/uucore/src/lib/mods/io.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// This file is part of the uutils coreutils package.
//
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.

#[cfg(not(windows))]
use std::os::fd::{AsFd, OwnedFd};
#[cfg(windows)]
use std::os::windows::io::{AsHandle, OwnedHandle};
use std::{
fs::{File, OpenOptions},
io::{self, Stdout},
path::Path,
process::Stdio,
};

#[cfg(windows)]
type XType = OwnedHandle;
#[cfg(not(windows))]
type XType = OwnedFd;

pub struct OwnedFileDescriptorOrHandle {
fx: XType,
}

impl OwnedFileDescriptorOrHandle {
pub fn new(x: XType) -> Self {
Self { fx: x }
}

pub fn open_file(options: &OpenOptions, path: &Path) -> io::Result<Self> {
let f = options.open(path)?;
Self::from(f)
}

#[cfg(windows)]
pub fn from<T: AsHandle>(t: T) -> io::Result<Self> {
Ok(Self {
fx: t.as_handle().try_clone_to_owned()?,
})
}

#[cfg(not(windows))]
pub fn from<T: AsFd>(t: T) -> io::Result<Self> {
Ok(Self {
fx: t.as_fd().try_clone_to_owned()?,
})
}

pub fn into_file(self) -> File {
File::from(self.fx)
}

pub fn into_stdio(self) -> Stdio {
Stdio::from(self.fx)
}

pub fn try_clone(&self) -> io::Result<Self> {
self.fx.try_clone().map(Self::new)
}

pub fn as_raw(&self) -> &XType {
&self.fx
}
}

impl From<OwnedFileDescriptorOrHandle> for Stdio {
fn from(value: OwnedFileDescriptorOrHandle) -> Self {
value.into_stdio()
}
}

impl TryFrom<&Stdout> for OwnedFileDescriptorOrHandle {
type Error = io::Error;

fn try_from(value: &Stdout) -> Result<Self, Self::Error> {

Check warning on line 76 in src/uucore/src/lib/mods/io.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/mods/io.rs#L76

Added line #L76 was not covered by tests
#[cfg(windows)]
let x = value.as_handle().try_clone_to_owned()?;
#[cfg(not(windows))]
let x = value.as_fd().try_clone_to_owned()?;
Ok(Self::new(x))
}

Check warning on line 82 in src/uucore/src/lib/mods/io.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/mods/io.rs#L81-L82

Added lines #L81 - L82 were not covered by tests
}
50 changes: 49 additions & 1 deletion tests/by-util/test_dd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
// spell-checker:ignore fname, tname, fpath, specfile, testfile, unspec, ifile, ofile, outfile, fullblock, urand, fileio, atoe, atoibm, availible, behaviour, bmax, bremain, btotal, cflags, creat, ctable, ctty, datastructures, doesnt, etoa, fileout, fname, gnudd, iconvflags, iseek, nocache, noctty, noerror, nofollow, nolinks, nonblock, oconvflags, oseek, outfile, parseargs, rlen, rmax, rposition, rremain, rsofar, rstat, sigusr, sigval, wlen, wstat abcdefghijklm abcdefghi nabcde nabcdefg abcdefg fifoname
// spell-checker:ignore fname, tname, fpath, specfile, testfile, unspec, ifile, ofile, outfile, fullblock, urand, fileio, atoe, atoibm, availible, behaviour, bmax, bremain, btotal, cflags, creat, ctable, ctty, datastructures, doesnt, etoa, fileout, fname, gnudd, iconvflags, iseek, nocache, noctty, noerror, nofollow, nolinks, nonblock, oconvflags, oseek, outfile, parseargs, rlen, rmax, rposition, rremain, rsofar, rstat, sigusr, sigval, wlen, wstat abcdefghijklm abcdefghi nabcde nabcdefg abcdefg fifoname seekable

#[cfg(unix)]
use crate::common::util::run_ucmd_as_root_with_stdin_stdout;
Expand All @@ -11,6 +11,7 @@ use crate::common::util::TestScenario;
use crate::common::util::{UCommand, TESTS_BINARY};

use regex::Regex;
use uucore::io::OwnedFileDescriptorOrHandle;

use std::fs::{File, OpenOptions};
use std::io::{BufReader, Read, Write};
Expand Down Expand Up @@ -1713,3 +1714,50 @@ fn test_reading_partial_blocks_from_fifo_unbuffered() {
let expected = b"0+2 records in\n0+2 records out\n4 bytes copied";
assert!(output.stderr.starts_with(expected));
}

#[test]
fn test_stdin_stdout_not_rewound_even_when_connected_to_seekable_file() {
use std::process::Stdio;

let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;

at.write("in", "abcde");

let stdin = OwnedFileDescriptorOrHandle::open_file(
OpenOptions::new().read(true),
at.plus("in").as_path(),
)
.unwrap();
let stdout = OwnedFileDescriptorOrHandle::open_file(
OpenOptions::new().create(true).write(true),
at.plus("out").as_path(),
)
.unwrap();
let stderr = OwnedFileDescriptorOrHandle::open_file(
OpenOptions::new().create(true).write(true),
at.plus("err").as_path(),
)
.unwrap();

ts.ucmd()
.args(&["bs=1", "skip=1", "count=1"])
.set_stdin(Stdio::from(stdin.try_clone().unwrap()))
.set_stdout(Stdio::from(stdout.try_clone().unwrap()))
.set_stderr(Stdio::from(stderr.try_clone().unwrap()))
.succeeds();

ts.ucmd()
.args(&["bs=1", "skip=1"])
.set_stdin(stdin)
.set_stdout(stdout)
.set_stderr(stderr)
.succeeds();

let err_file_content = std::fs::read_to_string(at.plus_as_string("err")).unwrap();
println!("stderr:\n{}", err_file_content);

let out_file_content = std::fs::read_to_string(at.plus_as_string("out")).unwrap();
println!("stdout:\n{}", out_file_content);
assert_eq!(out_file_content, "bde");
}

0 comments on commit 3de43ad

Please sign in to comment.