chcpu: report each error only once - #617
Open
mmclinton wants to merge 2 commits into
Open
Conversation
added 2 commits
August 5, 2026 20:23
Every failure was printed twice: `CpuList::run` reported it itself with a raw `eprintln!`, then also returned it, so `uucore` printed it again with the `chcpu: ` prefix. The unprefixed copy came first, which is the one users saw. Reporting each per-CPU failure with `uucore::show!` keeps the diagnostic for every element of a CPU list, gives all of them the program-name prefix that was previously only on the duplicate, and sets the exit code rather than returning an error - the pattern uucore documents for non-fatal errors when applying an operation to many items. Exit codes are untouched: 1 when nothing succeeded, 64 for partial success. Adds the first tests for this utility, covering the error paths. They need no privileges and change no CPU state: an absent CPU index is rejected before anything is written, and enabling an already-enabled CPU only prints.
These reach only clap, never sysfs, so they also guard the option surface on the platforms where the utility itself is unimplemented.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is my first contribution here, so please say if the scope or style should
be different and I will adjust.
The problem
chcpuprints every failure twice. The first copy has nochcpu:prefix, andwhen a CPU list has several failures only the first one is repeated:
With this change:
The partial-success path was wrong in a different way: it printed only the
unprefixed copy, so whether a diagnostic carried the program name depended on
which exit path was taken.
Why it happened
CpuList::runreported each failure itself with a plaineprintln!, and thenalso returned the first error. Returning it means
#[uucore::main]prints it asecond time, this time with the prefix.
Both halves were there for a reason: the
eprintln!so that every CPU in a listgets a diagnostic rather than just the first, and the returned error so the exit
status is non-zero. The fix keeps both of those properties.
The change
Each per-CPU failure is now reported with
uucore::show!, which printschcpu: <error>and sets the exit code, sorunno longer returns an error.This is the pattern uucore documents for non-fatal errors when an operation is
applied to many items.
Exit codes are unchanged: 1 when nothing succeeded, 64 for partial success.
There is no change to the CLI surface, so
chcpu.mdand--helpare untouched,and no new dependencies.
I also replaced the
foldwith aforloop, because the closure now performsI/O and a plain loop seemed easier to follow. That part is not required by the
fix, so I am happy to drop it if you would rather keep the diff smaller.
Tests
chcpuhad no test file, so this adds one and registers it intests/tests.rs.Ten tests in two groups:
Linux and also cover the macOS and Windows CI legs, where the utility itself
is unimplemented.
above.
The Linux tests need no privileges and change no CPU state. An out-of-range CPU
index is rejected before anything is written, and enabling an already-enabled
CPU returns before writing. The partial-success test looks up a hot-pluggable
CPU at runtime and skips if the machine has none, since
cpu0often has noonlineattribute.The suite goes from 163 tests to 173, and both commits pass fmt, clippy and the
tests on their own.
Note
This implementation also differs from util-linux 2.42.2 in its exit status for
some inputs, and in validating a CPU list as a whole rather than element by
element. Both are pre-existing and each is its own behavioural question, so I
left them out to keep this PR on the duplicated output. Happy to open an issue.