Skip to content

chown, chgrp: report a verbose stdout write error instead of panicking#13381

Open
leeewee wants to merge 1 commit into
uutils:mainfrom
leeewee:perms-fix-verbose-write-error
Open

chown, chgrp: report a verbose stdout write error instead of panicking#13381
leeewee wants to merge 1 commit into
uutils:mainfrom
leeewee:perms-fix-verbose-write-error

Conversation

@leeewee

@leeewee leeewee commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #10572

The verbose output in the shared ChownExecutor (uucore/src/lib/features/perms.rs) was printed with println!, which aborts (exit 134) when stdout can't be written — e.g. output redirected to a full device. Because the site is shared, both chown and chgrp abort, so this also covers the chgrp case of the same bug:

$ chown -v $(whoami) missing > /dev/full        # and: chgrp -v <group> missing > /dev/full
thread 'main' panicked at library/std/src/io/stdio.rs:...:
failed printing to stdout: No space left on device
Aborted (core dumped)          # exit 134

Verbose lines now go through a helper that propagates the write failure: on error it reports write error: <reason> to stderr and returns a non-zero code, matching GNU (exit 1). The three stdout println! sites (the obtain_metaNone branch in traverse, and both print_verbose_ownership_retained_as lines) use it, and print_verbose_ownership_retained_as now returns its exit code to its callers.

After:

$ chgrp -v <group> missing > /dev/full
chgrp: cannot dereference 'missing': No such file or directory
chgrp: write error: No space left on device          # exit 1

This matches GNU (exit 1, write error: No space left on device). Regression tests are added for both chgrp and chown (/dev/full, Linux-gated), each exercising the trigger with only stdout on the full sink.

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)

Comment thread src/uucore/src/lib/features/perms.rs Outdated
fn write_verbose_line(&self, line: &str) -> i32 {
use std::io::Write;
if let Err(e) = writeln!(std::io::stdout(), "{line}") {
show_error!("write error: {}", strip_errno(&e));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the translate! macro, and preferably a PermsError enum, like #13397.

The shared ChownExecutor in uucore/perms.rs printed verbose lines with
`println!`, which aborts (exit 134) when stdout can't be written, e.g.
`chown -v OWNER missing > /dev/full`. Because the site is shared, both
chown and chgrp abort.

Write verbose lines through a helper that propagates the failure: on a
write error it reports `write error: <reason>` to stderr and returns a
non-zero code, matching GNU (exit 1). The three stdout `println!` sites
(the obtain_meta-None branch and the two retained-as lines) now use it,
and `print_verbose_ownership_retained_as` returns its exit code to its
callers.

Fixes uutils#10572
@leeewee
leeewee force-pushed the perms-fix-verbose-write-error branch from 5dbb303 to d2d44ea Compare July 15, 2026 06:48
@leeewee

leeewee commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Updated per review:

  • Switched to the thiserror + translate! pattern — added a PermsError::Write(io::Error) enum reusing the existing common-write-error locale key (mirrors checksum/mod.rs), so write_verbose_line no longer hand-rolls the message.
  • While here, aligned the reporting with GNU's close_stdout: the write error is now detected once at a single final stdout().flush() in exec() and reported a single time (previously it printed once per failed verbose line). The two main.rs files use bin!(…, no_flush) so the util owns that flush and there's no extra Error flushing stdout: line. Verified byte-for-byte against GNU on /dev/full (exit 1, one write error: line).

@codspeed-hq

codspeed-hq Bot commented Jul 15, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 7.71%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 338 untouched benchmarks
⏩ 46 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation complex_relative_date 148 µs 137.4 µs +7.71%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing leeewee:perms-fix-verbose-write-error (d2d44ea) with main (2772896)

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.

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.

chown -v $(whoami) missing>/dev/full panics

2 participants