Skip to content

Conversation

@Qelxiros
Copy link
Contributor

closes #7166

Also changes number parsing to allow for a leading +, in line with GNU head

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@jfinkels
Copy link
Collaborator

I don't think this quite fixes the linked issue. I tried compiling uu_head on this branch and the error message is still displayed:

$ ./target/debug/head --bytes=-18446744073709551616000 < /dev/null
./target/debug/head: invalid number of bytes: '18446744073709551616000': Value too large for defined data type

@Qelxiros
Copy link
Contributor Author

Should be fixed now.

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/head/head-c is no longer failing!

@Qelxiros
Copy link
Contributor Author

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.

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/head/head-c is no longer failing!

Copy link
Collaborator

@jfinkels jfinkels left a 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> {
Copy link
Collaborator

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.

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/head/head-c is no longer failing!

@sylvestre sylvestre requested review from Copilot and jfinkels April 13, 2025 10:18
Copy link

Copilot AI left a 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()))),

@sylvestre sylvestre merged commit 4c796ca into uutils:main Apr 13, 2025
69 checks passed
@jfinkels
Copy link
Collaborator

Looks good to me, thanks.

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.

head: errors on very large argument to -c but shouldn't

3 participants