Skip to content

mknod: match GNU's error for a bad MAJOR/MINOR device number - #14316

Closed
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/mknod-major-minor-invalid-value
Closed

mknod: match GNU's error for a bad MAJOR/MINOR device number#14316
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/mknod-major-minor-invalid-value

Conversation

@arbelonson-source

Copy link
Copy Markdown
Contributor

What

MAJOR and MINOR validate their values with clap's own
value_parser!(u32), so an unparseable or overflowing value produces
clap's own generic wording instead of GNU's:

$ mknod f c 4294967296 1

# GNU
mknod: invalid major device number '4294967296'

# uutils, before this PR
error: invalid value '4294967296' for '[major]': 4294967296 is not in 0..=4294967295

Fix

Resolve the value by hand: parse as u32, reject anything that
doesn't fit (unparseable or overflowing) with GNU's wording,
distinguishing which of the two operands (major/minor) it was for --
same wording shape as ptx --gap-size/--width (#14315), no separate
"value too large" case needed since GNU's own message doesn't
distinguish overflow from any other bad value here either.

A negative value passed as its own positional argument (e.g. mknod f c -5 5) is deliberately left alone, still reported as clap's own
"unexpected argument" rejection: GNU's own getopt also rejects it,
but as an unrecognized option (invalid option -- '5') rather
than an invalid device number -- a different and much deeper mismatch
(clap's vs. getopt's own unknown-argument handling in general, not
anything specific to this option) that's out of scope for this fix.

Testing

  • cargo test --features mknod -p uu_mknod / full tests/by-util/test_mknod.rs suite: 14 passed, 0 failed.
  • Fixed 2 existing tests (3 assertions) that had encoded clap's wrong wording as the expected result, one with a comment explicitly noting it ("clap generated message, thats fine").
  • Manually diffed 7 cases (invalid, overflow, valid) on both major and minor, plus the negative-value positional-argument case, against GNU mknod 9.11 under LC_ALL=C (via sudo, since device-node creation requires it).
  • cargo clippy -p uu_mknod --all-targets -- -D warnings and cargo fmt --check: clean.

This PR was written with AI assistance (Claude Opus 5, via Claude Code). I've tested the changes but please review the code carefully.

MAJOR and MINOR validated their values with clap's own
value_parser!(u32), so an unparseable or overflowing value produced
clap's own generic wording ('invalid value ... invalid digit found in
string' / '... is not in 0..=4294967295') instead of GNU's uniform
'invalid major/minor device number: ...'.

Resolve the value by hand: parse as u32, reject anything that doesn't
fit (unparseable or overflowing) with GNU's wording, distinguishing
which of the two operands it was for. A negative value passed as its
own positional argument (e.g. 'mknod f c -5 5') is left as clap's own
'unexpected argument' rejection: GNU's own getopt also rejects it as
an unrecognized OPTION ('invalid option -- '5'') rather than an
invalid device number, a different and much deeper mismatch (clap vs.
getopt's own unknown-argument handling, not specific to this option)
that's out of scope for this fix.

AI-assisted-by: Claude Opus 5, via Claude Code
@oech3

oech3 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

clap's error message is better at here because it provides what you should do too.
@arbelonson-source Please stop removing clap provided error.

@sylvestre

Copy link
Copy Markdown
Contributor

i think our error msg is better, so, let's keep it.

@sylvestre sylvestre closed this Aug 31, 2026
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/rm/isatty (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/retry (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/date-locale-hour (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/rm/many-dir-entries-vs-OOM is now being skipped but was previously passing.
Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

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