-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
head: fix overflow errors #7721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
GNU testsuite comparison: |
e9002cc to
09ef12f
Compare
|
GNU testsuite comparison: |
|
I don't think this quite fixes the linked issue. I tried compiling |
|
Should be fixed now. |
|
GNU testsuite comparison: |
|
I don't have an easy way to test GNU behavior on 32-bit platforms, so if commit d887038 is a regression, let me know. |
|
GNU testsuite comparison: |
jfinkels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congrats! The gnu test tests/head/head-c is no longer failing!
Great!
| Ok(bytes_written) | ||
| } | ||
|
|
||
| fn catch_too_large_numbers_in_backwards_bytes_or_lines(n: u64) -> Option<usize> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just remove this helper function entirely and use the usize::try_from(n).ok() directly where needed.
d887038 to
37eee0e
Compare
|
GNU testsuite comparison: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
src/uu/head/src/parse.rs:20
- [nitpick] The variable 'plus_possible' may not clearly convey its intent; consider renaming it or adding an inline comment to clarify its purpose.
let mut plus_possible = false;
src/uu/head/src/parse.rs:101
- [nitpick] Using 'saturating_mul' changes the overflow behavior by masking potential errors; please confirm that this behavior is intended and that losing the overflow error signal is acceptable.
let num = num.saturating_mul(n);
src/uu/head/src/head.rs:194
- [nitpick] The consolidation of ParseError cases into a single error message may reduce clarity in error reporting; please verify that a unified message meets the expected behavior.
Some(Err(parse::ParseError)) => Err(HeadError::ParseError(format!("bad argument format: {}", s.quote()))),
|
Looks good to me, thanks. |
closes #7166
Also changes number parsing to allow for a leading
+, in line with GNUhead