Repository navigation
od, nl: match GNU on -w suffixes and huge -l values - #15564
Merged
Merged
Conversation
-w takes a plain decimal number. It was parsed like a size, so 1K, 2b or 0x8 were accepted and a leading zero meant octal.
A number too large for u64 now saturates instead of being rejected. -v, -i and -w still reject values that are too large.
sylvestre
reviewed
Oct 21, 2026
|
|
||
| /// Parses a plain decimal number, as `-w` takes: no radix prefix (a leading | ||
| /// zero is not octal) and no multiplier suffix. | ||
| pub fn parse_plain_number(s: &str) -> Result<u64, ParseSizeError> { |
Contributor
There was a problem hiding this comment.
seems like a too generic function name, maybe parse_width?
Contributor
Author
There was a problem hiding this comment.
Fair point, it was too generic for what it does. Renamed it to parse_width.
sylvestre
reviewed
Oct 21, 2026
| @@ -1335,14 +1335,12 @@ fn test_od_invalid_bytes() { | |||
| const BIG_SIZE: &str = "1Y"; | |||
|
|
|||
| // NOTE: | |||
Contributor
There was a problem hiding this comment.
do we still need this NOTE now that --width is gone from the list?
Contributor
Author
There was a problem hiding this comment.
You're right, with --width out of that list the note no longer says anything useful. Removed it.
Rename parse_plain_number to parse_width, drop the test_od note that no longer applies now that --width is out of the list, and stop piping stdin into the nl test whose options are rejected before any input is read (the write raced with nl exiting and hit a broken pipe on i686-musl).
…arguments These commands exit on the bad argument without reading stdin, so writing to the child's stdin raced with its exit and could fail with a broken pipe.
Contributor
|
Thanks for your PR |
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.
Two small option-parsing fixes, one commit each.
od -wwent through the same parser as-Nand-j, so it took size suffixes (1K,1b),0xhex, and read a leading zero as octal. GNU only takes a plain decimal number there.-wnow has its own small parser that accepts digits only and reuses the existing error messages. As a side effect-w010is now 10 instead of 8, and-w08is accepted, both matching GNU. The bare-wform and the huge-width path are untouched. The--width=1Ycase intest_od_invalid_bytesexpected the old behavior, so I moved that check into the new tests.nl -lrejected anything aboveu64::MAX. GNU accepts it, since it only caps how many blank lines get joined. A value that is all digits and too large now saturates tou64::MAX.-v,-iand-wstill reject large values, same as GNU.Added tests in
test_od.rsandtest_nl.rsplus a unit test for the new parser. Three of them fail on main and pass with this change. I also ran 24 cases against the GNU 9.12 binaries on macOS, and the exit code and output match on all of them.Left out on purpose, since main behaves the same way:
od -w+4,od -w' 4'andnl -l ' 3'are accepted by GNU but rejected here. The panic on a huge-wvalue belongs to #12885.Fixes #15551