df: round the total row once, from the summed bytes by TanbirRamim · Pull Request #14522 · uutils/coreutils

TanbirRamim · 2026-09-12T20:53:40Z

df --total summed the block counts of the rows, each already rounded up to the block size, instead of rounding the summed bytes once like GNU df does. With -B 100G on my machine GNU reports a total of 19 blocks where the rows add up to 24; we printed 24. With a block size larger than every filesystem the total degenerates into a count of filesystems, and -h then multiplied that count back by the block size and overflowed (attempt to multiply with overflow, exit 134).

This goes back to keeping the raw byte counts in the rows and rounding at output time. #10438 had moved away from that so the total would match the sum of the displayed rows, but GNU does not do that either, so test_total now allows for the rounding difference.

Fixes #14459

Copilot

🟡 Changes recommended

Unresolved moderate findings remain in total formatting and regression-test assertions.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates df --total to sum raw bytes before rounding, matching GNU behavior and avoiding large-block-size overflow.

Changes:

Accumulates raw byte totals and rounds once at output.
Adds regression coverage for rounding and human-readable large block sizes.
Updates integration expectations for per-row rounding differences.

File summaries

File	Summary
`tests/by-util/test_df.rs`	Updates total assertions and adds regression coverage; exact-value, deterministic-filesystem, and versioned GNU-reference adjustments remain requested.
`src/uu/df/src/table.rs`	Aggregates raw bytes and formats totals; explicit `-B` with `-h` still needs GNU-compatible precedence and output handling.

Review details

Suppressed comments (3)

src/uu/df/src/table.rs:270

When -h is combined with an explicit -B, this branch still formats the raw total as human-readable and ignores the requested block size. For the linked issue's reproducer GNU prints 1 block, while this remains an E-suffixed byte value; the new integration test only checks that it does not panic. Make the -B/-h precedence consistent with GNU and assert the resulting output.

        let s = if let Some(h) = self.options.human_readable {
            to_magnitude_and_suffix(bytes.into(), SuffixType::HumanReadable(h), true)
        } else {
            let BlockSize::Bytes(d) = self.options.block_size;
            bytes.div_ceil(d).to_string()

tests/by-util/test_df.rs:502

The regression test says the total should be one block, but <= 1 also accepts zero, so a broken or missing total accumulator could still pass. Assert the exact expected value.

    assert!(total <= 1, "total row reports {total} blocks");

tests/by-util/test_df.rs:490

This command sums every mounted filesystem, so a fixed 10-exabyte block size does not guarantee a one-block total: on a host whose included filesystems exceed 10 EB, correct output is 2 or more and this test fails. Restrict the command to a known filesystem (for example, append .) and assert the expected result, or use only the deterministic table fixture for this regression.

            "--block-size=10000000000000000000",
        ])

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟡 Changes recommended

A critical total-formatting issue and a regression-test coverage gap remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

tests/by-util/test_df.rs:496

This test only invokes df for ., so it normally has one filesystem row. The removed BytesCell implementation would also print 1 for both that row and the total, meaning this assertion does not exercise rounding across rows. Use a deterministic multi-filesystem fixture (or make the command-level test assert a real multi-row case) so the regression can fail again.

    let values: Vec<&str> = output.lines().skip(1).map(str::trim).collect();
    assert_eq!(values, vec!["1", "1"]);

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Lite

github-actions · 2026-09-12T22:50:18Z

GNU testsuite comparison:

Congrats! The gnu test tests/id/setgid is no longer failing!

codspeed · 2026-09-12T23:28:07Z

Merging this PR will not alter performance

1 warning, 1 note

⚠️ 121 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

ℹ️ 1 benchmark has no flamegraph

The flamegraph wasn't generated: most of the time was spent in runtime internals, which would leave nothing useful to analyze.

See affected benchmarks

✅ 401 untouched benchmarks
⏩ 70 skipped benchmarks¹

_{Comparing TanbirRamim:df-total-round-once (995c244) with main (1d6ca65)²}

70 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on main (e90b788) during the generation of this report, so 1d6ca65 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

cakebaker · 2026-09-13T13:00:07Z

Is it the expected behavior that the tests pass if I don't have applied your changes to table.rs?

No, thanks for checking. With only . there was a single row, so rounding once and summing rounded rows gave the same result. The tests now pass . .: without the table.rs changes test_total_rounds_up_once gets a total of 2 instead of 1 and test_total_human_readable_with_large_block_size panics with the multiply overflow.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

TanbirRamim · 2026-10-23T03:04:59Z

friendly bump, happy to change anything if needed

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 uutils#14459

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.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open issues

For --total -h --block-size=10000000000000000000 -a, this now formats the raw summed bytes, so it…
The helper is named filesystem_with_bytes, but its parameter is called blocks. The value is… · New

_{Review effort: Lite}

Copilot

🔵 Needs a closer look

It changes user-visible df --total output, including the -h total, and relaxes an existing test assertion, so a human should confirm the GNU behavior.

No open issues 🎉

2 resolved since last review

For --total -h --block-size=10000000000000000000 -a, this now formats the raw summed bytes, so it…
The helper is named filesystem_with_bytes, but its parameter is called blocks. The value is…

_{Review effort: Lite}

Copilot AI lite review requested due to automatic review settings September 12, 2026 20:53

Copilot started reviewing on behalf of TanbirRamim September 12, 2026 20:55

View session

Copilot AI reviewed Sep 12, 2026

View reviewed changes

Comment thread tests/by-util/test_df.rs

Copilot AI review requested due to automatic review settings September 12, 2026 21:19

Copilot started reviewing on behalf of TanbirRamim September 12, 2026 21:29

View session

Copilot AI reviewed Sep 12, 2026

View reviewed changes

Comment thread src/uu/df/src/table.rs

cakebaker reviewed Sep 13, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 19, 2026 18:26

Copilot AI reviewed Sep 19, 2026

Copilot AI review requested due to automatic review settings September 23, 2026 13:38

Copilot AI reviewed Sep 23, 2026

Copilot AI lite review requested due to automatic review settings September 29, 2026 20:45

Copilot AI reviewed Sep 29, 2026

TanbirRamim and others added 3 commits October 25, 2026 11:18

df: make the total-row regression tests exact

9f00aeb

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.

df: make --total regression tests use two rows

fd10a16

sylvestre force-pushed the df-total-round-once branch from b7a9c31 to fd10a16 Compare October 25, 2026 10:18

Copilot AI lite review requested due to automatic review settings October 25, 2026 10:18

Copilot AI reviewed Oct 25, 2026

View reviewed changes

Comment thread src/uu/df/src/table.rs Outdated

Copilot stopped reviewing on behalf of TanbirRamim due to an error October 25, 2026 11:43
The job was not acquired by a runner even after multiple attempts.

df: name the test helper parameter bytes

995c244

Copilot AI lite review requested due to automatic review settings October 25, 2026 21:17

Copilot started reviewing on behalf of TanbirRamim October 25, 2026 21:26

View session

Copilot AI reviewed Oct 25, 2026

View reviewed changes

Conversation

TanbirRamim commented Sep 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

github-actions Bot commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed Bot commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

cakebaker Sep 13, 2026

Choose a reason for hiding this comment

Uh oh!

TanbirRamim Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

TanbirRamim commented Oct 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 12, 2026 •

edited

Loading

codspeed Bot commented Sep 12, 2026 •

edited

Loading