dd: fix nocache flag handling at EOF (#9818)

* dd: fix nocache flag handling at EOF

* Add tests for nocache EOF handling

---------

Co-authored-by: Sylvestre Ledru <[email protected]>

Author: ChrisDryden

src/uu/dd/src/dd.rs

@@ -219,17 +219,6 @@ impl Source {
         Self::StdinFile(f)
     }
 
-    /// The length of the data source in number of bytes.
-    ///
-    /// If it cannot be determined, then this function returns 0.
-    fn len(&self) -> io::Result<i64> {
-        #[allow(clippy::match_wildcard_for_single_variants)]
-        match self {
-            Self::File(f) => Ok(f.metadata()?.len().try_into().unwrap_or(i64::MAX)),
-            _ => Ok(0),
-        }
-    }
-
     fn skip(&mut self, n: u64) -> io::Result<u64> {
         match self {
             #[cfg(not(unix))]
@@ -673,17 +662,6 @@ impl Dest {
             _ => Err(Errno::ESPIPE), // "Illegal seek"
         }
     }
-
-    /// The length of the data destination in number of bytes.
-    ///
-    /// If it cannot be determined, then this function returns 0.
-    fn len(&self) -> io::Result<i64> {
-        #[allow(clippy::match_wildcard_for_single_variants)]
-        match self {
-            Self::File(f, _) => Ok(f.metadata()?.len().try_into().unwrap_or(i64::MAX)),
-            _ => Ok(0),
-        }
-    }
 }
 
 /// Decide whether the given buffer is all zeros.
@@ -1063,21 +1041,12 @@ impl BlockWriter<'_> {
 /// depending on the command line arguments, this function
 /// informs the OS to flush/discard the caches for input and/or output file.
 fn flush_caches_full_length(i: &Input, o: &Output) -> io::Result<()> {
-    // TODO Better error handling for overflowing `len`.
+    // Using len=0 in posix_fadvise means "to end of file"
     if i.settings.iflags.nocache {
-        let offset = 0;
-        #[allow(clippy::useless_conversion)]
-        let len = i.src.len()?.try_into().unwrap();
-        i.discard_cache(offset, len);
+        i.discard_cache(0, 0);
     }
-    // Similarly, discard the system cache for the output file.
-    //
-    // TODO Better error handling for overflowing `len`.
     if i.settings.oflags.nocache {
-        let offset = 0;
-        #[allow(clippy::useless_conversion)]
-        let len = o.dst.len()?.try_into().unwrap();
-        o.discard_cache(offset, len);
+        o.discard_cache(0, 0);
     }
 
     Ok(())
@@ -1185,6 +1154,7 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> {
 
     let input_nocache = i.settings.iflags.nocache;
     let output_nocache = o.settings.oflags.nocache;
+    let output_direct = o.settings.oflags.direct;
 
     // Add partial block buffering, if needed.
     let mut o = if o.settings.buffered {
@@ -1208,6 +1178,12 @@ fn dd_copy(mut i: Input, o: Output) -> io::Result<()> {
         let loop_bsize = calc_loop_bsize(i.settings.count, &rstat, &wstat, i.settings.ibs, bsize);
         let rstat_update = read_helper(&mut i, &mut buf, loop_bsize)?;
         if rstat_update.is_empty() {
+            if input_nocache {
+                i.discard_cache(read_offset.try_into().unwrap(), 0);
+            }
+            if output_nocache || output_direct {
+                o.discard_cache(write_offset.try_into().unwrap(), 0);
+            }
             break;
         }
         let wstat_update = o.write_blocks(&buf)?;

tests/by-util/test_dd.rs

@@ -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 seekable
+// 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 fadvise FADV DONTNEED
 
 use uutests::at_and_ucmd;
 use uutests::new_ucmd;
@@ -1840,3 +1840,52 @@ fn test_skip_overflow() {
             "dd: invalid number: ‘9223372036854775808’: Value too large for defined data type",
         );
 }
+
+#[test]
+#[cfg(target_os = "linux")]
+fn test_nocache_eof() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.write_bytes("in.f", &vec![0u8; 1234567]);
+    ucmd.args(&[
+        "if=in.f",
+        "of=out.f",
+        "bs=1M",
+        "oflag=nocache,sync",
+        "status=noxfer",
+    ])
+    .succeeds();
+    assert_eq!(at.read_bytes("out.f").len(), 1234567);
+}
+
+#[test]
+#[cfg(all(target_os = "linux", feature = "printf"))]
+fn test_nocache_eof_fadvise_zero_length() {
+    use std::process::Command;
+    let (at, _ucmd) = at_and_ucmd!();
+    at.write_bytes("in.f", &vec![0u8; 1234567]);
+
+    let strace_file = at.plus_as_string("strace.out");
+    let result = Command::new("strace")
+        .args(["-o", &strace_file, "-e", "fadvise64,fadvise64_64"])
+        .arg(get_tests_binary())
+        .args([
+            "dd",
+            "if=in.f",
+            "of=out.f",
+            "bs=1M",
+            "oflag=nocache,sync",
+            "status=none",
+        ])
+        .current_dir(at.as_string())
+        .output();
+
+    if result.is_err() {
+        return; // strace not available
+    }
+
+    let strace = at.read("strace.out");
+    assert!(
+        strace.contains(", 0, POSIX_FADV_DONTNEED"),
+        "Expected len=0 at EOF: {strace}"
+    );
+}