From c20394e52323e1aa96cbd7ae639207556c86722e 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/2] 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 71cc83ee57..562f13e4cd 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -18,7 +18,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. /// @@ -38,13 +38,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. /// @@ -81,9 +81,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, @@ -114,13 +114,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")] @@ -138,7 +138,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, @@ -163,9 +163,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 { @@ -192,48 +192,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. @@ -304,18 +262,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) } @@ -356,9 +308,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 => { @@ -490,7 +442,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 { @@ -577,7 +529,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() { @@ -613,9 +565,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")] @@ -779,9 +731,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() @@ -806,9 +758,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() @@ -855,7 +807,7 @@ mod tests { ..Default::default() }; let row = Row { - bytes: BytesCell::new(100, &BlockSize::Bytes(100)), + bytes: 100, inodes: 10, ..Default::default() }; @@ -876,9 +828,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() @@ -911,9 +863,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() @@ -958,9 +910,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() @@ -986,6 +938,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(); @@ -1013,7 +1037,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 98b2981ff1..92fa72dca9 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -462,10 +462,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 8ff66fb09fbd5eef3bd9c893144719c0f2545530 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/2] 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 92fa72dca9..89f6c0fc43 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -463,8 +463,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), @@ -480,32 +480,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(); }