From 60e9812bf118c691ed275b9b6ad09b7087c0accd Mon Sep 17 00:00:00 2001 From: Zhang Wen Date: Tue, 23 Jun 2026 15:05:27 +0800 Subject: [PATCH 1/2] add DdError for better error handling --- src/uu/dd/locales/en-US.ftl | 2 ++ src/uu/dd/src/dd.rs | 35 +++++++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/uu/dd/locales/en-US.ftl b/src/uu/dd/locales/en-US.ftl index 3b72e4a8f6c..b42a1133e6b 100644 --- a/src/uu/dd/locales/en-US.ftl +++ b/src/uu/dd/locales/en-US.ftl @@ -128,6 +128,8 @@ dd-error-cannot-skip-invalid = '{ $file }': cannot skip: Invalid argument dd-error-cannot-seek-invalid = '{ $output }': cannot seek: Invalid argument dd-error-not-directory = setting flags for '{ $file }': Not a directory dd-error-failed-discard-cache = failed to discard cache for: { $file } +dd-error-general-io = general io error: {$error} +dd-error-cannot-skip-offset-infile = { $file }: cannot skip to specified offset # Parse errors dd-error-unrecognized-operand = Unrecognized operand '{ $operand }' diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 782d2664c21..3e92f0529a9 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -48,8 +48,9 @@ use std::time::{Duration, Instant}; use clap::{Arg, Command}; use gcd::Gcd; +use thiserror::Error; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult}; +use uucore::error::{FromIo, UError, UResult}; #[cfg(unix)] use uucore::error::{USimpleError, set_exit_code}; #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] @@ -167,6 +168,20 @@ impl Num { } } +#[derive(Error, Debug)] +enum DdError { + #[error("{}", translate!("dd-error-general-io", "error" => _0))] + IOError(#[from] io::Error), + #[error("{}", .0)] + OtherError(io::Error), +} + +impl UError for DdError { + fn code(&self) -> i32 { + 1 + } +} + /// Read and discard `n` bytes from `reader` using a buffer of size `buf_size`. /// /// This is more efficient than `io::copy` with `BufReader` because it reads @@ -1112,7 +1127,7 @@ fn flush_caches_full_length(i: &Input, o: &Output) { /// /// If there is a problem reading from the input or writing to /// this output. -fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { +fn dd_copy(mut i: Input, o: Output) -> Result<(), DdError> { // The read and write statistics. // // These objects are counters, initialized to zero. After each @@ -1169,7 +1184,8 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { &prog_tx, output_thread, truncate, - ); + ) + .map_err(Into::into); } // Spawn a timer thread to provide a scheduled signal indicating when we @@ -1210,7 +1226,8 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { // Create a common empty buffer with a capacity of the block size. // This is the max size needed. let mut buf = Vec::new(); - buf.try_reserve(bsize)?; // try_with_capacity is unstable https://github.com/rust-lang/rust/issues/91913 + buf.try_reserve(bsize) + .map_err(|e| DdError::OtherError(e.into()))?; // try_with_capacity is unstable https://github.com/rust-lang/rust/issues/91913 // The main read/write loop. // @@ -1287,7 +1304,7 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { } } - finalize(o, rstat, wstat, start, &prog_tx, output_thread, truncate) + finalize(o, rstat, wstat, start, &prog_tx, output_thread, truncate).map_err(DdError::IOError) } /// Flush output, print final stats, and join with the progress thread. @@ -1538,7 +1555,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { 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(|| translate!("dd-error-io-error")) + match dd_copy(i, o) { + Ok(_) => Ok(()), + Err(DdError::IOError(e)) => Err(e.into()), + Err(DdError::OtherError(e)) => { + Err(e).map_err_context(|| translate!("dd-error-write-error")) + } + } } pub fn uu_app() -> Command { From 31993a7bc888272b92a2a1f0e90b2cce9a505146 Mon Sep 17 00:00:00 2001 From: Zhang Wen Date: Wed, 24 Jun 2026 15:05:52 +0800 Subject: [PATCH 2/2] fix stdout output --- src/uu/dd/src/bufferedoutput.rs | 4 +-- src/uu/dd/src/dd.rs | 47 ++++++++++++++++++--------------- tests/by-util/test_dd.rs | 13 +++++++++ 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/src/uu/dd/src/bufferedoutput.rs b/src/uu/dd/src/bufferedoutput.rs index cd16eb0e701..c1b94051928 100644 --- a/src/uu/dd/src/bufferedoutput.rs +++ b/src/uu/dd/src/bufferedoutput.rs @@ -8,7 +8,7 @@ //! //! Use the [`BufferedOutput`] struct to create a buffered form of the //! [`Output`] writer. -use crate::{Output, WriteStat}; +use crate::{DdError, Output, WriteStat}; /// Buffer partial output blocks until they are completed. /// @@ -57,7 +57,7 @@ impl<'a> BufferedOutput<'a> { } /// Truncate the underlying file to the current stream position, if possible. - pub(crate) fn truncate(&mut self) -> std::io::Result<()> { + pub(crate) fn truncate(&mut self) -> Result<(), DdError> { self.inner.dst.truncate() } diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 3e92f0529a9..5fcb249e978 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -660,9 +660,11 @@ impl Dest { } #[cfg_attr(not(unix), allow(unused_variables))] - fn seek(&mut self, n: u64, obs: usize) -> io::Result { + fn seek(&mut self, n: u64, obs: usize) -> Result { match self { - Self::Stdout(stdout) => io::copy(&mut io::repeat(0).take(n), stdout), + Self::Stdout(stdout) => { + io::copy(&mut io::repeat(0).take(n), stdout).map_err(DdError::IOError) + } Self::File(f, _) => { #[cfg(unix)] if let Ok(Some(len)) = try_get_len_of_block_device(f) @@ -677,12 +679,18 @@ impl Dest { set_exit_code(1); return Ok(len); } - f.seek(SeekFrom::Current(n.try_into().unwrap())) + f.seek(SeekFrom::Current(n.try_into().map_err(|e| { + DdError::IOError(io::Error::new( + io::ErrorKind::InvalidInput, + format!("invalid input: {e:?}"), + )) + })?)) + .map_err(DdError::IOError) } #[cfg(unix)] Self::Fifo(f) => { // Seeking in a named pipe means *reading* from the pipe. - read_and_discard(f, n, obs) + read_and_discard(f, n, obs).map_err(DdError::IOError) } #[cfg(unix)] Self::Sink => Ok(0), @@ -690,18 +698,18 @@ impl Dest { } /// Truncate the underlying file to the current stream position, if possible. - fn truncate(&mut self) -> io::Result<()> { + fn truncate(&mut self) -> Result<(), DdError> { #[allow(clippy::match_wildcard_for_single_variants)] match self { Self::File(f, _) => { - let pos = f.stream_position()?; + let pos = f.stream_position().unwrap_or_default(); // `set_len()` can fail with EINVAL on special outputs such as // `/dev/null`; GNU `dd` ignores that. But on a regular file a // truncate failure (e.g. ENOSPC, read-only fs) means silent data // loss, so the error must surface there. match f.set_len(pos) { Ok(()) => Ok(()), - Err(e) if f.metadata().is_ok_and(|m| m.file_type().is_file()) => Err(e), + Err(e) if f.metadata().is_ok_and(|m| m.file_type().is_file()) => Err(e.into()), Err(_) => Ok(()), } } @@ -849,8 +857,7 @@ impl<'a> Output<'a> { fn new_stdout(settings: &'a Settings) -> UResult { let fx = OwnedFileDescriptorOrHandle::from(io::stdout())?; let mut dst = Dest::Stdout(fx.into_file()); - dst.seek(settings.seek, settings.obs) - .map_err_context(|| translate!("dd-error-write-error"))?; + dst.seek(settings.seek, settings.obs)?; Ok(Self { dst, settings }) } @@ -887,19 +894,18 @@ impl<'a> Output<'a> { dst.set_len(settings.seek).ok(); } - Self::prepare_file(dst, settings) + Ok(Self::prepare_file(dst, settings)) } - fn prepare_file(dst: File, settings: &'a Settings) -> UResult { + fn prepare_file(dst: File, settings: &'a Settings) -> Self { let density = if settings.oconv.sparse { Density::Sparse } else { Density::Dense }; let mut dst = Dest::File(dst, density); - dst.seek(settings.seek, settings.obs) - .map_err_context(|| translate!("dd-error-failed-to-seek"))?; - Ok(Self { dst, settings }) + dst.seek(settings.seek, settings.obs).unwrap_or_default(); + Self { dst, settings } } /// Instantiate this struct with file descriptor as a destination. @@ -918,7 +924,7 @@ impl<'a> Output<'a> { .map_err(|e| uucore::error::UIoError::from(io::Error::from(e)))?; } - Self::prepare_file(fx.into_file(), settings) + Ok(Self::prepare_file(fx.into_file(), settings)) } /// Instantiate this struct with the given named pipe as a destination. @@ -1044,7 +1050,7 @@ impl<'a> Output<'a> { } /// Truncate the underlying file to the current stream position, if possible. - fn truncate(&mut self) -> io::Result<()> { + fn truncate(&mut self) -> Result<(), DdError> { self.dst.truncate() } } @@ -1089,7 +1095,7 @@ impl BlockWriter<'_> { /// Errors are suppressed for special outputs (e.g. `/dev/null`) but /// propagated for regular files, so a failed truncate does not silently /// leave stale data behind. See [`Dest::truncate`]. - fn truncate(&mut self) -> io::Result<()> { + fn truncate(&mut self) -> Result<(), DdError> { match self { Self::Unbuffered(o) => o.truncate(), Self::Buffered(o) => o.truncate(), @@ -1184,8 +1190,7 @@ fn dd_copy(mut i: Input, o: Output) -> Result<(), DdError> { &prog_tx, output_thread, truncate, - ) - .map_err(Into::into); + ); } // Spawn a timer thread to provide a scheduled signal indicating when we @@ -1304,7 +1309,7 @@ fn dd_copy(mut i: Input, o: Output) -> Result<(), DdError> { } } - finalize(o, rstat, wstat, start, &prog_tx, output_thread, truncate).map_err(DdError::IOError) + finalize(o, rstat, wstat, start, &prog_tx, output_thread, truncate) } /// Flush output, print final stats, and join with the progress thread. @@ -1316,7 +1321,7 @@ fn finalize( prog_tx: &mpsc::Sender, output_thread: thread::JoinHandle, truncate: bool, -) -> io::Result<()> { +) -> Result<(), DdError> { // Flush the output in case a partial write has been buffered but // not yet written. let wstat_update = output.flush()?; diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 309a93ac020..6c5b5db4ab8 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -2128,3 +2128,16 @@ fn test_bs_not_positive() { } } } + +/* + * Test that the output file can be `/dev/stdout`. + * on android, we don't have permission to /dev/stdout + */ +#[cfg(all(unix, not(target_os = "android")))] +#[test] +fn test_outfile_dev_stdout() { + new_ucmd!() + .args(&["if=/dev/null", "of=/dev/stdout"]) + .succeeds() + .no_stdout(); +}