From a375644c502463c6e0d31156d7e6b615ded4d815 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 11 Jun 2022 22:41:54 -0400 Subject: [PATCH] dd: only print concise byte counts if large enough Update `dd` to only print a concise form of the number of bytes with an SI prefix (like "1 MB" or "2 GB") if the number is at least 1000. Similarly, only print the concise form with an IEC prefix (like "1 MiB" or "2 GiB") if the number is at least 1024. For example, $ head -c 999 /dev/zero | dd > /dev/null 1+1 records in 1+1 records out 999 bytes copied, 0.0 s, 999.0 KB/s $ head -c 1000 /dev/zero | dd > /dev/null 1+1 records in 1+1 records out 1000 bytes (1000 B) copied, 0.0 s, 1000.0 KB/s $ head -c 1024 /dev/zero | dd > /dev/null 2+0 records in 2+0 records out 1024 bytes (1 KB, 1024 B) copied, 0.0 s, 1.0 MB/s --- src/uu/dd/src/progress.rs | 97 +++++++++++++++++++++++++++++---------- tests/by-util/test_dd.rs | 46 ++++++++++--------- 2 files changed, 96 insertions(+), 47 deletions(-) diff --git a/src/uu/dd/src/progress.rs b/src/uu/dd/src/progress.rs index 46369d291..315fb862a 100644 --- a/src/uu/dd/src/progress.rs +++ b/src/uu/dd/src/progress.rs @@ -118,10 +118,7 @@ impl ProgUpdate { /// let mut cursor = Cursor::new(vec![]); /// let rewrite = false; /// prog_update.write_prog_line(&mut cursor, rewrite).unwrap(); - /// assert_eq!( - /// cursor.get_ref(), - /// b"0 bytes (0 B, 0 B) copied, 1.0 s, 0 B/s\n" - /// ); + /// assert_eq!(cursor.get_ref(), b"0 bytes copied, 1.0 s, 0 B/s\n"); /// ``` fn write_prog_line(&self, w: &mut impl Write, rewrite: bool) -> std::io::Result<()> { let btotal_metric = Byte::from_bytes(self.write_stat.bytes_total) @@ -137,17 +134,38 @@ impl ProgUpdate { let btotal = self.write_stat.bytes_total; let duration = self.duration.as_secs_f64(); - if rewrite { + + // If we are rewriting the progress line, do write a carriage + // return (`\r`) at the beginning and don't write a newline + // (`\n`) at the end. + let (carriage_return, newline) = if rewrite { ("\r", "") } else { ("", "\n") }; + + // If the number of bytes written is sufficiently large, then + // print a more concise representation of the number, like + // "1.2 kB" and "1.0 KiB". + if btotal < 1000 { write!( w, - "\r{} bytes ({}, {}) copied, {:.1} s, {}/s", - btotal, btotal_metric, btotal_bin, duration, transfer_rate + "{}{} bytes copied, {:.1} s, {}/s{}", + carriage_return, btotal, duration, transfer_rate, newline, + ) + } else if btotal < 1024 { + write!( + w, + "{}{} bytes ({}) copied, {:.1} s, {}/s{}", + carriage_return, btotal, btotal_metric, duration, transfer_rate, newline, ) } else { - writeln!( + write!( w, - "{} bytes ({}, {}) copied, {:.1} s, {}/s", - btotal, btotal_metric, btotal_bin, duration, transfer_rate + "{}{} bytes ({}, {}) copied, {:.1} s, {}/s{}", + carriage_return, + btotal, + btotal_metric, + btotal_bin, + duration, + transfer_rate, + newline, ) } } @@ -176,10 +194,7 @@ impl ProgUpdate { /// let mut iter = cursor.get_ref().split(|v| *v == b'\n'); /// assert_eq!(iter.next().unwrap(), b"0+0 records in"); /// assert_eq!(iter.next().unwrap(), b"0+0 records out"); - /// assert_eq!( - /// iter.next().unwrap(), - /// b"0 bytes (0 B, 0 B) copied, 1.0 s, 0 B/s" - /// ); + /// assert_eq!(iter.next().unwrap(), b"0 bytes copied, 1.0 s, 0 B/s"); /// assert_eq!(iter.next().unwrap(), b""); /// assert!(iter.next().is_none()); /// ``` @@ -437,6 +452,17 @@ mod tests { use super::{ProgUpdate, ReadStat, WriteStat}; + fn prog_update_write(n: u128) -> ProgUpdate { + ProgUpdate { + read_stat: Default::default(), + write_stat: WriteStat { + bytes_total: n, + ..Default::default() + }, + duration: Duration::new(1, 0), // one second + } + } + #[test] fn test_read_stat_report() { let read_stat = ReadStat::new(1, 2, 3); @@ -474,12 +500,7 @@ mod tests { #[test] fn test_prog_update_write_prog_line() { - let prog_update = ProgUpdate { - read_stat: Default::default(), - write_stat: Default::default(), - duration: Duration::new(1, 0), // one second - }; - + let prog_update = prog_update_write(0); let mut cursor = Cursor::new(vec![]); let rewrite = false; prog_update.write_prog_line(&mut cursor, rewrite).unwrap(); @@ -489,9 +510,38 @@ mod tests { // $ : | dd // 0 bytes copied, 7.9151e-05 s, 0.0 kB/s // + // The throughput still does not match GNU dd. For the ones + // that include the concise byte counts, the format is also + // not right. + assert_eq!(cursor.get_ref(), b"0 bytes copied, 1.0 s, 0 B/s\n"); + + let prog_update = prog_update_write(999); + let mut cursor = Cursor::new(vec![]); + prog_update.write_prog_line(&mut cursor, rewrite).unwrap(); + assert_eq!(cursor.get_ref(), b"999 bytes copied, 1.0 s, 0 B/s\n"); + + let prog_update = prog_update_write(1000); + let mut cursor = Cursor::new(vec![]); + prog_update.write_prog_line(&mut cursor, rewrite).unwrap(); assert_eq!( cursor.get_ref(), - b"0 bytes (0 B, 0 B) copied, 1.0 s, 0 B/s\n" + b"1000 bytes (1000 B) copied, 1.0 s, 1000 B/s\n" + ); + + let prog_update = prog_update_write(1023); + let mut cursor = Cursor::new(vec![]); + prog_update.write_prog_line(&mut cursor, rewrite).unwrap(); + assert_eq!( + cursor.get_ref(), + b"1023 bytes (1 KB) copied, 1.0 s, 1000 B/s\n" + ); + + let prog_update = prog_update_write(1024); + let mut cursor = Cursor::new(vec![]); + prog_update.write_prog_line(&mut cursor, rewrite).unwrap(); + assert_eq!( + std::str::from_utf8(cursor.get_ref()).unwrap(), + "1024 bytes (1 KB, 1024 B) copied, 1.0 s, 1000 B/s\n" ); } @@ -507,10 +557,7 @@ mod tests { let mut iter = cursor.get_ref().split(|v| *v == b'\n'); assert_eq!(iter.next().unwrap(), b"0+0 records in"); assert_eq!(iter.next().unwrap(), b"0+0 records out"); - assert_eq!( - iter.next().unwrap(), - b"0 bytes (0 B, 0 B) copied, 1.0 s, 0 B/s" - ); + assert_eq!(iter.next().unwrap(), b"0 bytes copied, 1.0 s, 0 B/s"); assert_eq!(iter.next().unwrap(), b""); assert!(iter.next().is_none()); } diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 3cc5346b7..dffc5a01a 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -248,17 +248,10 @@ fn test_final_stats_noxfer() { #[test] fn test_final_stats_unspec() { - let output = vec![ - "0+0 records in", - "0+0 records out", - "0 bytes (0 B, 0 B) copied, 0.0 s, 0 B/s", - ]; - let output = output.into_iter().fold(String::new(), |mut acc, s| { - acc.push_str(s); - acc.push('\n'); - acc - }); - new_ucmd!().run().stderr_only(&output).success(); + new_ucmd!() + .run() + .stderr_only("0+0 records in\n0+0 records out\n0 bytes copied, 0.0 s, 0 B/s\n") + .success(); } #[cfg(any(target_os = "linux", target_os = "android"))] @@ -370,20 +363,10 @@ fn test_existing_file_truncated() { #[test] fn test_null_stats() { - let stats = vec![ - "0+0 records in\n", - "0+0 records out\n", - "0 bytes (0 B, 0 B) copied, 0.0 s, 0 B/s\n", - ]; - let stats = stats.into_iter().fold(String::new(), |mut acc, s| { - acc.push_str(s); - acc - }); - new_ucmd!() .args(&["if=null.txt"]) .run() - .stderr_only(stats) + .stderr_only("0+0 records in\n0+0 records out\n0 bytes copied, 0.0 s, 0 B/s\n") .success(); } @@ -1184,3 +1167,22 @@ fn test_bytes_oseek_seek_additive() { .succeeds() .stdout_is_fixture_bytes("dd-bytes-alphabet-null.spec"); } + +#[test] +fn test_final_stats_si_iec() { + let result = new_ucmd!().pipe_in("0".repeat(999)).succeeds(); + let s = result.stderr_str(); + assert!(s.starts_with("1+1 records in\n1+1 records out\n999 bytes copied,")); + + let result = new_ucmd!().pipe_in("0".repeat(1000)).succeeds(); + let s = result.stderr_str(); + assert!(s.starts_with("1+1 records in\n1+1 records out\n1000 bytes (1000 B) copied,")); + + let result = new_ucmd!().pipe_in("0".repeat(1023)).succeeds(); + let s = result.stderr_str(); + assert!(s.starts_with("1+1 records in\n1+1 records out\n1023 bytes (1 KB) copied,")); + + let result = new_ucmd!().pipe_in("0".repeat(1024)).succeeds(); + let s = result.stderr_str(); + assert!(s.starts_with("2+0 records in\n2+0 records out\n1024 bytes (1 KB, 1024 B) copied,")); +}