From 1522091a4fcb203a2f82cb1e942b3989c9555ab4 Mon Sep 17 00:00:00 2001 From: Zhiwei Liang Date: Fri, 9 Oct 2026 04:05:02 -0400 Subject: [PATCH] dd: ignore a failed cache drop while copying Signed-off-by: Zhiwei Liang --- src/uu/dd/src/bufferedoutput.rs | 4 +- src/uu/dd/src/dd.rs | 102 ++++++++++++++++---------------- tests/by-util/test_dd.rs | 31 ++++++++++ 3 files changed, 83 insertions(+), 54 deletions(-) diff --git a/src/uu/dd/src/bufferedoutput.rs b/src/uu/dd/src/bufferedoutput.rs index 4803f667415..804f406446e 100644 --- a/src/uu/dd/src/bufferedoutput.rs +++ b/src/uu/dd/src/bufferedoutput.rs @@ -41,8 +41,8 @@ impl<'a> BufferedOutput<'a> { Ok(Self { inner, buf }) } - pub(crate) fn discard_cache(&self, offset: u64, len: u64) { - self.inner.discard_cache(offset, len); + pub(crate) fn discard_cache(&self, offset: u64, len: u64) -> std::io::Result<()> { + self.inner.discard_cache(offset, len) } /// Flush the partial block stored in the internal buffer. diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 12d8194dd58..7f23b27fa16 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -53,7 +53,6 @@ use uucore::display::Quotable; #[cfg(unix)] use uucore::error::USimpleError; use uucore::error::{FromIo, UResult, set_exit_code}; -#[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] use uucore::show_if_err; use uucore::{format_usage, show_error}; @@ -330,6 +329,14 @@ impl Source { _ => Err(rustix::io::Errno::SPIPE.into()), } } + + // Not reached today: the parser accepts `nocache` only on Linux and + // Android, and FreeBSD builds the fadvise variant above. Kept so that + // every caller sees one signature. + #[cfg(not(any(target_os = "linux", target_os = "android", target_os = "freebsd")))] + fn discard_cache(&self, _offset: u64, _len: u64) -> io::Result<()> { + Err(io::ErrorKind::Unsupported.into()) + } } impl Read for Source { @@ -495,28 +502,10 @@ impl Input<'_> { /// /// `offset` and `len` specify a contiguous portion of the input. /// This function informs the kernel that the specified portion of - /// the input file is no longer needed. If not possible, then this - /// function prints an error message to stderr and sets the exit - /// status code to 1. - #[cfg_attr( - not(any(target_os = "linux", target_os = "android", target_os = "freebsd")), - allow(clippy::unused_self, unused_variables) - )] - fn discard_cache(&self, offset: u64, len: u64) { - #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] - { - let file = self - .settings - .infile - .clone() - .unwrap_or_else(|| translate!("dd-standard-input")); - show_if_err!( - self.src.discard_cache(offset, len).map_err_context( - || translate!("dd-error-failed-discard-cache", "file" => file) - ) - ); - } - // TODO: Is there a way to discard filesystem cache on other targets? + /// the input file is no longer needed. If not possible, the error is + /// returned; the caller decides whether to report it. + fn discard_cache(&self, offset: u64, len: u64) -> io::Result<()> { + self.src.discard_cache(offset, len) } /// Fills a given buffer. @@ -726,6 +715,14 @@ impl Dest { _ => Err(rustix::io::Errno::SPIPE.into()), } } + + // Not reached today: the parser accepts `nocache` only on Linux and + // Android, and FreeBSD builds the fadvise variant above. Kept so that + // every caller sees one signature. + #[cfg(not(any(target_os = "linux", target_os = "android", target_os = "freebsd")))] + fn discard_cache(&self, _offset: u64, _len: u64) -> io::Result<()> { + Err(io::ErrorKind::Unsupported.into()) + } } /// Decide whether the given buffer is all zeros. @@ -954,28 +951,10 @@ impl<'a> Output<'a> { /// /// `offset` and `len` specify a contiguous portion of the output. /// This function informs the kernel that the specified portion of - /// the output file is no longer needed. If not possible, then - /// this function prints an error message to stderr and sets the - /// exit status code to 1. - #[cfg_attr( - not(any(target_os = "linux", target_os = "android", target_os = "freebsd")), - allow(clippy::unused_self, unused_variables) - )] - fn discard_cache(&self, offset: u64, len: u64) { - #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] - { - let file = self - .settings - .outfile - .clone() - .unwrap_or_else(|| translate!("dd-standard-output")); - show_if_err!( - self.dst.discard_cache(offset, len).map_err_context( - || translate!("dd-error-failed-discard-cache", "file" => file) - ) - ); - } - // TODO Is there a way to discard filesystem cache on other targets? + /// the output file is no longer needed. If not possible, the error is + /// returned; the caller decides whether to report it. + fn discard_cache(&self, offset: u64, len: u64) -> io::Result<()> { + self.dst.discard_cache(offset, len) } /// writes a block of data. optionally retries when first try didn't complete @@ -1063,7 +1042,7 @@ enum BlockWriter<'a> { } impl BlockWriter<'_> { - fn discard_cache(&self, offset: u64, len: u64) { + fn discard_cache(&self, offset: u64, len: u64) -> io::Result<()> { match self { Self::Unbuffered(o) => o.discard_cache(offset, len), Self::Buffered(o) => o.discard_cache(offset, len), @@ -1111,10 +1090,26 @@ impl BlockWriter<'_> { fn flush_caches_full_length(i: &Input, o: &Output) { // Using len=0 in posix_fadvise means "to end of file" if i.settings.iflags.nocache { - i.discard_cache(0, 0); + let file = i + .settings + .infile + .clone() + .unwrap_or_else(|| translate!("dd-standard-input")); + show_if_err!( + i.discard_cache(0, 0) + .map_err_context(|| translate!("dd-error-failed-discard-cache", "file" => file)) + ); } if i.settings.oflags.nocache { - o.discard_cache(0, 0); + let file = o + .settings + .outfile + .clone() + .unwrap_or_else(|| translate!("dd-standard-output")); + show_if_err!( + o.discard_cache(0, 0) + .map_err_context(|| translate!("dd-error-failed-discard-cache", "file" => file)) + ); } } @@ -1253,11 +1248,14 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { break; }; if rstat_update.is_empty() { + // Dropping the cache is best-effort: a failure (a pipe cannot + // drop its cache at all) must not fail or diagnose a copy that + // succeeded. Only the `count=0` path reports it. if input_nocache { - i.discard_cache(read_offset, 0); + let _ = i.discard_cache(read_offset, 0); } if output_nocache || output_direct { - o.discard_cache(write_offset.try_into().unwrap(), 0); + let _ = o.discard_cache(write_offset.try_into().unwrap(), 0); } break; } @@ -1277,7 +1275,7 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { if input_nocache { let offset = read_offset; let len = read_len; - i.discard_cache(offset, len); + let _ = i.discard_cache(offset, len); } read_offset += read_len; @@ -1289,7 +1287,7 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> { if output_nocache { let offset = write_offset.try_into().unwrap(); let len = write_len.try_into().unwrap(); - o.discard_cache(offset, len); + let _ = o.discard_cache(offset, len); } write_offset += write_len; diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index f47bf4ffc3c..857a7ea25c3 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -1801,6 +1801,23 @@ fn test_nocache_stdin_error() { .stderr_only(format!("dd: failed to discard cache for: 'standard input': {detail}\n0+0 records in\n0+0 records out\n")); } +/// Test that discarding system file cache fails for an output FIFO. +#[test] +#[cfg(target_os = "linux")] +fn test_nocache_output_fifo_error() { + #[cfg(not(target_env = "musl"))] + let detail = "Illegal seek"; + #[cfg(target_env = "musl")] + let detail = "Invalid seek"; + let (at, mut ucmd) = at_and_ucmd!(); + at.mkfifo("fifo"); + ucmd.args(&["oflag=nocache", "count=0", "of=fifo", "status=noxfer"]) + .fails_with_code(1) + .stderr_only(format!( + "dd: failed to discard cache for: fifo: {detail}\n0+0 records in\n0+0 records out\n" + )); +} + /// Test that dd fails when no number in count. #[test] fn test_empty_count_number() { @@ -2103,6 +2120,20 @@ fn test_iflag_directory_fails_when_file_is_piped_via_std_in() { .stderr_only("dd: setting flags for 'standard input': Not a directory\n"); } +#[test] +#[cfg(any(target_os = "linux", target_os = "android"))] +fn test_nocache_on_a_pipe_is_silent() { + // "" ends the copy on the first read, "abc" goes through the per-block + // path; neither may diagnose the failed cache drop on a pipe + for input in ["", "abc"] { + new_ucmd!() + .args(&["iflag=nocache", "oflag=nocache", "status=none"]) + .pipe_in(input) + .succeeds() + .stdout_only(input); + } +} + #[test] fn test_stdin_stdout_not_rewound_even_when_connected_to_seekable_file() { let ts = TestScenario::new(util_name!());