who: report a stdout write error instead of panicking#13389
Conversation
|
GNU testsuite comparison: |
| translate!("who-long-usage", "default_file" => utmpx::DEFAULT_FILE) | ||
| } | ||
|
|
||
| fn write_error(e: std::io::Error) -> Box<dyn UError> { |
`who -q` printed the user list and count with `println!`, which aborts (exit 134) when stdout can't be written, e.g. `who -q > /dev/full`. Write through a helper that propagates the failure: on error it reports a GNU-style `write error: <reason>` to stderr and exits 1. The non-`-q` record path already propagated its write error but reported it without the `write error:` prefix; route it through the same helper so both paths match GNU. Fixes uutils#13388
b758bcd to
de85c9d
Compare
| } | ||
|
|
||
| #[derive(Debug, Error)] | ||
| enum WhoError { |
There was a problem hiding this comment.
Are more variants expected? Otherwise a struct seems more appropriate imo..
There was a problem hiding this comment.
We generally define a UtilError enum for each utility. In the case of who, I do expect additional variants in the future, for example, a who-canonicalize-error variant.
| #[derive(Debug, Error)] | ||
| enum WhoError { | ||
| #[error("{}", translate!("who-error-write", "error" => strip_errno(.0)))] | ||
| Write(std::io::Error), |
There was a problem hiding this comment.
Maybe it could derive from?
| translate!("who-user-count", "count" => users.len()) | ||
| ) | ||
| }) | ||
| .map_err(WhoError::Write)?; |
There was a problem hiding this comment.
Then the map_err can be skipped
There was a problem hiding this comment.
Thanks! I tried this, but it regresses the output. who's print functions return UResult<()> (Result<(), Box<dyn UError>>), and uucore has a blanket impl From<io::Error> for Box<dyn UError> that builds a UIoError. So with #[from], a bare ? takes that one-step io::Error → Box<dyn UError> conversion and bypasses WhoError — the message drops from who: write error: No space left on device back to who: No space left on device. The explicit .map_err(WhoError::Write)? is what keeps the write-error path going through WhoError and its who-error-write message.
There was a problem hiding this comment.
I would prefer that the error variant is specified explicitly. It’s more future-proof in case we ever add a Read(std::io::Error) variant.
Merging this PR will not alter performance
Comparing 
Footnotes
|
4 participants
Fixes #13388
who -q(--count) printed the user list and the trailing count withprintln!, which aborts (exit 134) when stdout can't be written — e.g. output redirected to a full device. GNU reports the write error and exits 1.Both stdout output paths now go through a helper that propagates the failure: on error it reports a GNU-style
write error: <reason>to stderr and exits 1.Before:
After:
The non-
-qrecord-listing path already propagated its write error but reported it without thewrite error:prefix (who: No space left on device); it now routes through the same helper, so both paths match GNU (who: write error: No space left on device, exit 1).Same write-error class as chcon (#13061), kill (#13297), and runcon. The existing
--heading/dev/fulltest is updated to the new message, and a-qregression test is added.