Skip to content

ls: round block counts up when scaling to a block size - #14330

Open
Socialpranker wants to merge 1 commit into
uutils:mainfrom
Socialpranker:ls-block-size-round-up
Open

ls: round block counts up when scaling to a block size#14330
Socialpranker wants to merge 1 commit into
uutils:mainfrom
Socialpranker:ls-block-size-round-up

Conversation

@Socialpranker

Copy link
Copy Markdown
Contributor

ls -s --block-size=1000 on a 4096-byte allocation printed 4 blocks. A block
that is only partly used still occupies a whole block, so it is 5:

$ ls -s --block-size=1000 dir     # GNU
total 13
5 a1
5 a2
5 a3
$ ls -s --block-size=1000 dir     # uutils, before
total 12
4 a1
4 a2
4 a3

Every block size that is not a divisor of the allocation was off the same way:
--block-size=M on a 4 KiB file printed 0, --block-size=3000 printed 1
where it should print 2. --block-size=512, 2048 and the default 1024
divide evenly on a 4 KiB allocation, which is why the existing tests never
caught it.

The total line

total had a second defect on top of the first. It summed the already scaled
per-file figures, so the rounding error was repeated once per entry:

$ ls -s --block-size=1000 dir     # three 4096-byte allocations
total 13                          # GNU: ceil(12288 / 1000)
total 15                          # what per-file rounding would give

GNU adds up the allocated bytes and scales the sum once. Rounding up per file
and then adding, which is what a one-line fix in the scaling function alone
would produce, overshoots by up to one block per entry.

So the two steps are split apart: get_block_bytes reports the allocation in
bytes, scale_block_bytes scales a byte count to the block size and rounds up.
The per-file cell scales one file; total scales the sum. -h/--si pass the
byte count through, as before, since the human-readable formatter scales it
itself.

Non-unix has no allocation figure and falls back to the file size, which was
never scaled here. scale_block_bytes leaves it alone, so that path is
unchanged; cargo check --target x86_64-pc-windows-gnu is clean.

How the GNU behavior was established

By running the installed GNU binary as a black box and recording its output --
the rounding rule and the total semantics above are both read off the
transcripts shown here. I did not read GNU coreutils source.

Testing

  • cargo test --features ls --no-default-features -- test_ls: 160 passed, 0
    failed
    (159 pre-existing, 1 new). No existing test needed a change.
  • New test_ls_block_size_rounds_up derives its expectations from the
    filesystem's own blocks() rather than assuming a 4 KiB granularity, and
    returns early when the allocation divides evenly, so it cannot fail spuriously
    on a filesystem with a different allocation unit.
  • Mutation check: reverting ls.rs alone (test file untouched) makes the new
    test fail.
  • cargo fmt --all --check and cargo clippy -p uu_ls --all-targets: clean.
  • Differential A/B against GNU coreutils 9.7 in debian:stable-slim, 17 block
    sizes x 9 flag combinations over a directory of 6 files, a subdirectory, a
    symlink and a FIFO: 63 cases moved from differing to byte-identical, 0
    regressions
    . The LS_BLOCK_SIZE, BLOCK_SIZE and BLOCKSIZE paths match
    GNU too, where they did not before.

The 27 cases that still differ are all suffix-only block sizes (K, KB, M),
where GNU prints the unit with the number (4K, not 4). That is a separate
defect in a different place -- the same one #13655 is fixing for du -- and the
figures themselves now agree there as well.

Disclosure

Prepared with AI assistance (Claude Opus 5, via Claude Code), per the AI policy
in CONTRIBUTING.md. Every GNU behavior quoted above came from running the
installed binary, not from reading GPL source. All testing was run locally.

A partly used block still occupies a whole block, so scaling an
allocation to the block size rounds up. `ls -s --block-size=1000` on a
4096-byte allocation reported 4 blocks instead of 5, and every
non-divisor block size was off the same way.

The `total` line had a second, related defect: it summed the already
scaled per-file figures, so the rounding error was repeated once per
entry. It now sums the allocated bytes and scales the sum once.

Splits the two steps apart: `get_block_bytes` reports the allocation in
bytes and `scale_block_bytes` scales it, rounding up. Non-unix keeps its
file-size fallback unscaled, as before.
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/dd/misc. tests/dd/misc is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/df/over-mount-device. tests/df/over-mount-device is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/tail-n0f (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/cat/splice is no longer failing!
Congrats! The gnu test tests/cp/cp-a-selinux is no longer failing!
Congrats! The gnu test tests/cut/cut is no longer failing!
Congrats! The gnu test tests/cut/mb-non-utf8 is no longer failing!
Congrats! The gnu test tests/dd/partial-write is no longer failing!
Congrats! The gnu test tests/expand/mb is no longer failing!
Congrats! The gnu test tests/ls/stat-free-symlinks is no longer failing!
Congrats! The gnu test tests/misc/close-stdout is no longer failing!
Congrats! The gnu test tests/mktemp/write-error is no longer failing!
Congrats! The gnu test tests/mv/dir2dir is no longer failing!
Congrats! The gnu test tests/mv/mv-exchange is no longer failing!
Congrats! The gnu test tests/nl/multibyte is no longer failing!
Congrats! The gnu test tests/od/od-float is no longer failing!
Congrats! The gnu test tests/od/od-j is no longer failing!
Congrats! The gnu test tests/ptx/ptx-overrun is no longer failing!
Congrats! The gnu test tests/sort/sort-merge-fdlimit is no longer failing!
Congrats! The gnu test tests/unexpand/mb is no longer failing!
Note: The gnu test tests/dd/fail-ftruncate-fstat was skipped on 'main' but is now failing.

@sylvestre

Copy link
Copy Markdown
Contributor

Differential A/B against GNU coreutils 9.7

it is old, please try with master binaries
(or at least 9.11)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants