From d8847a4cb2b5f8554385b3b492d08c1c8820ae47 Mon Sep 17 00:00:00 2001 From: Tanbir Hossain Ramim <96797470+TanbirRamim@users.noreply.github.com> Date: Sat, 12 Sep 2026 22:50:40 +0200 Subject: [PATCH 1/4] df: round the total row once, from the summed bytes The total row added up the block counts of the rows, each of which is already rounded up to the block size, instead of rounding the summed bytes once the way GNU df does. With a block size larger than the filesystems, every non-empty row is one block, so the total became a count of filesystems, and the human-readable output then multiplied that count back by the block size and overflowed. Keep the raw byte counts in the rows, sum those for the total row, and round at output time like every other row. Fixes #14459 --- src/uu/df/src/table.rs | 204 ++++++++++++++++++++++----------------- tests/by-util/test_df.rs | 49 +++++++++- 2 files changed, 159 insertions(+), 94 deletions(-) diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index e168a04cbd3..e893e0b34a2 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -21,7 +21,7 @@ use uucore::translate; use std::ffi::OsString; use std::iter; -use std::ops::{Add, AddAssign}; +use std::ops::AddAssign; /// A row in the filesystem usage data table. /// @@ -41,13 +41,13 @@ pub(crate) struct Row { fs_mount: OsString, /// Total number of bytes in the filesystem regardless of whether they are used. - bytes: BytesCell, + bytes: u64, /// Number of used bytes. - bytes_used: BytesCell, + bytes_used: u64, /// Number of available bytes. - bytes_avail: BytesCell, + bytes_avail: u64, /// Percentage of bytes that are used, given as a float between 0 and 1. /// @@ -84,9 +84,9 @@ impl Row { fs_device: source.into(), fs_type: "-".into(), fs_mount: "-".into(), - bytes: BytesCell::default(), - bytes_used: BytesCell::default(), - bytes_avail: BytesCell::default(), + bytes: 0, + bytes_used: 0, + bytes_avail: 0, bytes_usage: None, #[cfg(target_vendor = "apple")] bytes_capacity: None, @@ -117,13 +117,13 @@ impl AddAssign for Row { bytes, bytes_used, bytes_avail, - bytes_usage: if bytes.bytes == 0 { + bytes_usage: if bytes == 0 { None } else { // We use "(bytes_used + bytes_avail)" instead of "bytes" because on some filesystems (e.g. // ext4) "bytes" also includes reserved blocks we ignore for the usage calculation. // https://www.gnu.org/software/coreutils/faq/coreutils-faq.html#df-Size-and-Used-and-Available-do-not-add-up - Some(bytes_used.bytes as f64 / (bytes_used.bytes + bytes_avail.bytes) as f64) + Some(bytes_used as f64 / (bytes_used + bytes_avail) as f64) }, // TODO Figure out how to compute this. #[cfg(target_vendor = "apple")] @@ -141,7 +141,7 @@ impl AddAssign for Row { } impl Row { - fn from_filesystem(fs: Filesystem, row_block_size: &BlockSize) -> Self { + fn from_filesystem(fs: Filesystem) -> Self { let MountInfo { dev_name, fs_type, @@ -166,9 +166,9 @@ impl Row { fs_device: dev_name, fs_type, fs_mount: mount_dir, - bytes: BytesCell::new(blocks * blocksize, row_block_size), - bytes_used: BytesCell::new(bused * blocksize, row_block_size), - bytes_avail: BytesCell::new(bavail * blocksize, row_block_size), + bytes: blocks * blocksize, + bytes_used: bused * blocksize, + bytes_avail: bavail * blocksize, bytes_usage: if blocks == 0 { None } else { @@ -195,48 +195,6 @@ impl Row { } } -#[derive(Debug, Copy, Clone)] -struct BytesCell { - bytes: u64, - scaled: u64, -} - -/// A bytes column in the filesystem usage data table. -/// -/// This is used to keep track of the scaled values to properly compute -/// the total values. -impl Default for BytesCell { - fn default() -> Self { - Self { - bytes: 0, - scaled: 0, - } - } -} - -impl BytesCell { - fn new(bytes: u64, block_size: &BlockSize) -> Self { - Self { - bytes, - scaled: { - let BlockSize::Bytes(d) = block_size; - (bytes as f64 / *d as f64).ceil() as u64 - }, - } - } -} - -impl Add for BytesCell { - type Output = Self; - - fn add(self, rhs: Self) -> Self { - Self { - bytes: self.bytes + rhs.bytes, - scaled: self.scaled + rhs.scaled, - } - } -} - /// A `Cell` in the table. We store raw `bytes` as the data (e.g. directory name /// may be non-Unicode). We also record the printed `width` for alignment purpose, /// as it is easier to compute on the original string. @@ -307,18 +265,12 @@ impl<'a> RowFormatter<'a> { /// Get a string giving the scaled version of the input number. /// /// The scaling factor is defined in the `options` field. - fn scaled_bytes(&self, bytes_column: &BytesCell) -> Cell { - let size = bytes_column.scaled; + fn scaled_bytes(&self, bytes: u64) -> Cell { let s = if let Some(h) = self.options.human_readable { - let size = if self.is_total_row { - let BlockSize::Bytes(d) = self.options.block_size; - d * size - } else { - bytes_column.bytes - }; - to_magnitude_and_suffix(size.into(), SuffixType::HumanReadable(h), true) + to_magnitude_and_suffix(bytes.into(), SuffixType::HumanReadable(h), true) } else { - size.to_string() + let BlockSize::Bytes(d) = self.options.block_size; + bytes.div_ceil(d).to_string() }; Cell::from_ascii_string(s) } @@ -359,9 +311,9 @@ impl<'a> RowFormatter<'a> { Cell::from_string(&self.row.fs_device) } } - Column::Size => self.scaled_bytes(&self.row.bytes), - Column::Used => self.scaled_bytes(&self.row.bytes_used), - Column::Avail => self.scaled_bytes(&self.row.bytes_avail), + Column::Size => self.scaled_bytes(self.row.bytes), + Column::Used => self.scaled_bytes(self.row.bytes_used), + Column::Avail => self.scaled_bytes(self.row.bytes_avail), Column::Pcent => Self::percentage(self.row.bytes_usage), Column::Target => { @@ -488,7 +440,7 @@ impl Table { // showing all filesystems, then print the data as a row in // the output table. if options.show_all_fs || filesystem.usage.blocks > 0 { - let row = Row::from_filesystem(filesystem, &options.block_size); + let row = Row::from_filesystem(filesystem); let fmt = RowFormatter::new(&row, options, false); let values = fmt.get_cells(); if options.show_total { @@ -576,7 +528,7 @@ mod tests { use crate::blocks::HumanReadable; use crate::columns::Column; - use crate::table::{BytesCell, Cell, Header, HeaderMode, Row, RowFormatter, Table}; + use crate::table::{Cell, Header, HeaderMode, Row, RowFormatter, Table}; use crate::{BlockSize, Options}; fn init() { @@ -612,9 +564,9 @@ mod tests { fs_type: "my_type".to_string(), fs_mount: "my_mount".into(), - bytes: BytesCell::new(100, &BlockSize::Bytes(1)), - bytes_used: BytesCell::new(25, &BlockSize::Bytes(1)), - bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1)), + bytes: 100, + bytes_used: 25, + bytes_avail: 75, bytes_usage: Some(0.25), #[cfg(target_vendor = "apple")] @@ -778,9 +730,9 @@ mod tests { fs_device: "my_device".to_string(), fs_mount: "my_mount".into(), - bytes: BytesCell::new(100, &BlockSize::Bytes(1)), - bytes_used: BytesCell::new(25, &BlockSize::Bytes(1)), - bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1)), + bytes: 100, + bytes_used: 25, + bytes_avail: 75, bytes_usage: Some(0.25), ..Default::default() @@ -805,9 +757,9 @@ mod tests { fs_type: "my_type".to_string(), fs_mount: "my_mount".into(), - bytes: BytesCell::new(100, &BlockSize::Bytes(1)), - bytes_used: BytesCell::new(25, &BlockSize::Bytes(1)), - bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1)), + bytes: 100, + bytes_used: 25, + bytes_avail: 75, bytes_usage: Some(0.25), ..Default::default() @@ -854,7 +806,7 @@ mod tests { ..Default::default() }; let row = Row { - bytes: BytesCell::new(100, &BlockSize::Bytes(100)), + bytes: 100, inodes: 10, ..Default::default() }; @@ -875,9 +827,9 @@ mod tests { fs_type: "my_type".to_string(), fs_mount: "my_mount".into(), - bytes: BytesCell::new(40000, &BlockSize::default()), - bytes_used: BytesCell::new(1000, &BlockSize::default()), - bytes_avail: BytesCell::new(39000, &BlockSize::default()), + bytes: 40000, + bytes_used: 1000, + bytes_avail: 39000, bytes_usage: Some(0.025), ..Default::default() @@ -910,9 +862,9 @@ mod tests { fs_type: "my_type".to_string(), fs_mount: "my_mount".into(), - bytes: BytesCell::new(4096, &BlockSize::default()), - bytes_used: BytesCell::new(1024, &BlockSize::default()), - bytes_avail: BytesCell::new(3072, &BlockSize::default()), + bytes: 4096, + bytes_used: 1024, + bytes_avail: 3072, bytes_usage: Some(0.25), ..Default::default() @@ -957,9 +909,9 @@ mod tests { }; let row = Row { - bytes: BytesCell::new(bytes, &BlockSize::Bytes(1000)), - bytes_used: BytesCell::new(bytes_used, &BlockSize::Bytes(1000)), - bytes_avail: BytesCell::new(bytes_avail, &BlockSize::Bytes(1000)), + bytes, + bytes_used, + bytes_avail, ..Default::default() }; RowFormatter::new(&row, &options, false).get_cells() @@ -985,6 +937,78 @@ mod tests { )); } + fn filesystem_with_bytes(blocks: u64) -> crate::Filesystem { + crate::Filesystem { + file: None, + mount_info: crate::MountInfo { + dev_id: "28".to_string(), + dev_name: "none".to_string(), + fs_type: "9p".to_string(), + mount_dir: "/mnt".into(), + mount_option: "rw".to_string(), + mount_root: "/".into(), + remote: false, + dummy: false, + }, + usage: crate::table::FsUsage { + blocksize: 1, + blocks, + bfree: 0, + bavail: 0, + bavail_top_bit_set: false, + files: 1, + ffree: 0, + }, + } + } + + fn last_line(table: &Table) -> Vec { + let mut data: Vec = vec![]; + table.write_to(&mut data).expect("Write error."); + let output = String::from_utf8_lossy(&data); + output + .lines() + .last() + .unwrap() + .split_whitespace() + .map(str::to_string) + .collect() + } + + #[test] + fn test_total_row_rounds_the_summed_bytes_once() { + init(); + // Two filesystems of 1500 bytes are 2 blocks each with 1000-byte + // blocks, but together they hold 3000 bytes, which is 3 blocks, not + // the sum of the rounded-up row values. + let options = Options { + show_total: true, + block_size: BlockSize::Bytes(1000), + columns: vec![Column::Source, Column::Size, Column::Used, Column::Avail], + ..Default::default() + }; + let filesystems = vec![filesystem_with_bytes(1500), filesystem_with_bytes(1500)]; + let table = Table::new(&options, filesystems); + assert_eq!(last_line(&table), vec!["total", "3", "3", "0"]); + } + + #[test] + fn test_total_row_human_readable_with_large_block_size() { + init(); + // The block size does not take part in human-readable output, so a + // block size close to `u64::MAX` must not overflow the total row. + let options = Options { + show_total: true, + human_readable: Some(HumanReadable::Binary), + block_size: BlockSize::Bytes(10_000_000_000_000_000_000), + columns: vec![Column::Source, Column::Size, Column::Used, Column::Avail], + ..Default::default() + }; + let filesystems = vec![filesystem_with_bytes(1500), filesystem_with_bytes(1500)]; + let table = Table::new(&options, filesystems); + assert_eq!(last_line(&table), vec!["total", "3.0K", "3.0K", "0"]); + } + #[test] fn test_row_converter_with_invalid_numbers() { init(); @@ -1012,7 +1036,7 @@ mod tests { }, }; - let row = Row::from_filesystem(d, &BlockSize::default()); + let row = Row::from_filesystem(d); assert_eq!(row.inodes_used, 0); } diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 7f4ab559960..f69e5eb45fb 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -464,10 +464,51 @@ fn test_total() { computed_total_avail += iter.next().unwrap().parse::().unwrap(); } - // Check that the sum of each column matches the reported value in the last row. - assert_eq!(computed_total_size, reported_total_size); - assert_eq!(computed_total_used, reported_total_used); - assert_eq!(computed_total_avail, reported_total_avail); + // The total row is the sum of the raw byte counts rounded up once, like + // GNU df does, while every other row is rounded up on its own. So the sum + // of the rows can exceed the total by less than one block per row. + let rows = (n - 1) as u64; + for (computed, reported) in [ + (computed_total_size, reported_total_size), + (computed_total_used, reported_total_used), + (computed_total_avail, reported_total_avail), + ] { + assert!( + reported <= computed && computed - reported < rows.max(1), + "total {reported} is not the rounded sum of {rows} rows summing to {computed}" + ); + } +} + +#[test] +fn test_total_rounds_up_once() { + // With a block size larger than any filesystem, every non-empty row rounds + // up to a single block, but the total is still one block, not one per row. + let output = new_ucmd!() + .args(&[ + "--total", + "--output=size", + "--block-size=10000000000000000000", + ]) + .succeeds() + .stdout_str_lossy(); + let total: u64 = output + .lines() + .last() + .unwrap() + .split_whitespace() + .last() + .unwrap() + .parse() + .unwrap(); + assert!(total <= 1, "total row reports {total} blocks"); +} + +#[test] +fn test_total_human_readable_with_large_block_size() { + new_ucmd!() + .args(&["--total", "-h", "--block-size=10000000000000000000"]) + .succeeds(); } /// Test that the "total" label appears in the correct column. From 9f00aeb780c75bbe99521545726facfac7d0ed77 Mon Sep 17 00:00:00 2001 From: Tanbir Hossain Ramim <96797470+TanbirRamim@users.noreply.github.com> Date: Sat, 12 Sep 2026 23:19:11 +0200 Subject: [PATCH 2/4] df: make the total-row regression tests exact Run them on the filesystem of the current directory only, so the one row and the total are both exactly one block, and name the GNU version the rounding was compared against. --- tests/by-util/test_df.rs | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index f69e5eb45fb..c6002f45862 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -465,8 +465,8 @@ fn test_total() { } // The total row is the sum of the raw byte counts rounded up once, like - // GNU df does, while every other row is rounded up on its own. So the sum - // of the rows can exceed the total by less than one block per row. + // GNU df (9.11) does, while every other row is rounded up on its own. So + // the sum of the rows can exceed the total by less than one block per row. let rows = (n - 1) as u64; for (computed, reported) in [ (computed_total_size, reported_total_size), @@ -482,32 +482,27 @@ fn test_total() { #[test] fn test_total_rounds_up_once() { - // With a block size larger than any filesystem, every non-empty row rounds - // up to a single block, but the total is still one block, not one per row. + // With a block size larger than the filesystem, the one row rounds up to a + // single block and so does the total: it is rounded once from the summed + // bytes, as GNU df (9.11) does, not summed from the rounded-up rows. let output = new_ucmd!() .args(&[ "--total", "--output=size", "--block-size=10000000000000000000", + ".", ]) .succeeds() .stdout_str_lossy(); - let total: u64 = output - .lines() - .last() - .unwrap() - .split_whitespace() - .last() - .unwrap() - .parse() - .unwrap(); - assert!(total <= 1, "total row reports {total} blocks"); + let values: Vec<&str> = output.lines().skip(1).map(str::trim).collect(); + assert_eq!(values, vec!["1", "1"]); } #[test] fn test_total_human_readable_with_large_block_size() { + // Used to multiply the rounded total back by the block size and overflow. new_ucmd!() - .args(&["--total", "-h", "--block-size=10000000000000000000"]) + .args(&["--total", "-h", "--block-size=10000000000000000000", "."]) .succeeds(); } From fd10a1635b53525232d60228ba03a88fb1d44c09 Mon Sep 17 00:00:00 2001 From: Tanbir Date: Wed, 23 Sep 2026 15:38:13 +0200 Subject: [PATCH 3/4] df: make --total regression tests use two rows --- tests/by-util/test_df.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index c6002f45862..aa263470e53 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -482,27 +482,35 @@ fn test_total() { #[test] fn test_total_rounds_up_once() { - // With a block size larger than the filesystem, the one row rounds up to a - // single block and so does the total: it is rounded once from the summed - // bytes, as GNU df (9.11) does, not summed from the rounded-up rows. + // With a block size larger than the filesystem, each of the two rows + // rounds up to a single block, but the total is rounded once from the + // summed bytes, as GNU df (9.11) does, so it is 1 and not 2. let output = new_ucmd!() .args(&[ "--total", "--output=size", "--block-size=10000000000000000000", ".", + ".", ]) .succeeds() .stdout_str_lossy(); let values: Vec<&str> = output.lines().skip(1).map(str::trim).collect(); - assert_eq!(values, vec!["1", "1"]); + assert_eq!(values, vec!["1", "1", "1"]); } #[test] fn test_total_human_readable_with_large_block_size() { - // Used to multiply the rounded total back by the block size and overflow. + // Used to multiply the summed rounded-up rows (2 here) back by the block + // size, which overflowed. new_ucmd!() - .args(&["--total", "-h", "--block-size=10000000000000000000", "."]) + .args(&[ + "--total", + "-h", + "--block-size=10000000000000000000", + ".", + ".", + ]) .succeeds(); } From 995c244b24c0e0912fbd031028bedf8f5de993f6 Mon Sep 17 00:00:00 2001 From: Tanbir Hossain Ramim <96797470+TanbirRamim@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:17:31 +0100 Subject: [PATCH 4/4] df: name the test helper parameter bytes --- src/uu/df/src/table.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index e893e0b34a2..7127b8d5702 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -937,7 +937,7 @@ mod tests { )); } - fn filesystem_with_bytes(blocks: u64) -> crate::Filesystem { + fn filesystem_with_bytes(bytes: u64) -> crate::Filesystem { crate::Filesystem { file: None, mount_info: crate::MountInfo { @@ -952,7 +952,7 @@ mod tests { }, usage: crate::table::FsUsage { blocksize: 1, - blocks, + blocks: bytes, bfree: 0, bavail: 0, bavail_top_bit_set: false,