Skip to content

cksum: get rid of print! and println! to avoid panicking on write errors#12099

Merged
sylvestre merged 2 commits into
uutils:mainfrom
RenjiSann:cksum-fix-print
May 4, 2026
Merged

cksum: get rid of print! and println! to avoid panicking on write errors#12099
sylvestre merged 2 commits into
uutils:mainfrom
RenjiSann:cksum-fix-print

Conversation

@RenjiSann

Copy link
Copy Markdown
Collaborator

Fix for #10947

Also needed to do some shenanigans in the uucore::bin macro to avoid having an extra flush failing.

@github-actions

github-actions Bot commented Apr 30, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/misc/io-errors. tests/misc/io-errors is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/rm/many-dir-entries-vs-OOM is now passing!

Comment thread src/uucore/src/lib/features/checksum/compute.rs Outdated
@RenjiSann RenjiSann force-pushed the cksum-fix-print branch 2 times, most recently from 355ddc8 to ad9b814 Compare April 30, 2026 20:51
@codspeed-hq

codspeed-hq Bot commented Apr 30, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 16.98%

❌ 1 regressed benchmark
✅ 310 untouched benchmarks
⏩ 46 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation hostname_ip_lookup[100000] 90.4 µs 108.9 µs -16.98%

Comparing RenjiSann:cksum-fix-print (72c09c8) with main (45a48a1)

Open in CodSpeed

Footnotes

  1. 46 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.

@RenjiSann

Copy link
Copy Markdown
Collaborator Author

Note that GNU seems to inconsistenly handle whether it shows the No space left on device detail depending on the presence of the -z flag (which replace line breaks by nul bytes).

I assume this is because the way the error is displayed is different between the writes and the final flush that happens in -z mode. I don't think it's relevant to follow this behavior here.

$ uu_cksum /dev/null > /dev/full
cksum: write error: No space left on device

$ gnu_cksum /dev/null > /dev/full       
cksum: write error                               # <-- GNU most probably prints the error differently here

$ uu_cksum /dev/null > /dev/full -z
cksum: write error: No space left on device

$ gnu_cksum /dev/null > /dev/full -z       
cksum: write error: No space left on device

@xtqqczze

Copy link
Copy Markdown
Contributor

$ gnu_cksum /dev/null > /dev/full
cksum: write error # <-- GNU most probably prints the error differently here

maybe you should report that to the coreutils/coreutils repo

Comment thread src/uucore/src/lib/features/checksum/compute.rs Outdated
@xtqqczze

Copy link
Copy Markdown
Contributor

some shenanigans in the uucore::bin macro to avoid having an extra flush failing.

Good work with the macros, I'd been wondering whether it was possible to avoid that flush.

Comment thread src/uucore/src/lib/features/checksum/compute.rs Outdated
@RenjiSann

Copy link
Copy Markdown
Collaborator Author

$ gnu_cksum /dev/null > /dev/full
cksum: write error # <-- GNU most probably prints the error differently here

maybe you should report that to the coreutils/coreutils repo

Done ! coreutils/coreutils#258

some shenanigans in the uucore::bin macro to avoid having an extra flush failing.

Good work with the macros, I'd been wondering whether it was possible to avoid that flush.

Thanks :)
It's a bit rough but it does not make a too big change. If we need to do the same thing for other binaries, maybe I'll try to find a cleaner way to do it ^^

@RenjiSann RenjiSann force-pushed the cksum-fix-print branch 2 times, most recently from 7e4e632 to d836b22 Compare May 4, 2026 09:06
@RenjiSann RenjiSann requested a review from sylvestre May 4, 2026 12:10
@sylvestre sylvestre merged commit 379fbbb into uutils:main May 4, 2026
184 of 191 checks passed
@RenjiSann RenjiSann deleted the cksum-fix-print branch May 4, 2026 13:21
naoNao89 added a commit to naoNao89/coreutils that referenced this pull request May 6, 2026
Adds a Linux-only regression test that verifies cksum exits with
code 1 (rather than panicking) when stdout is /dev/full.

Refs uutils#10947 (fixed by uutils#12099).
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