Skip to content

printf: make negative values wrap around with unsigned/hex format #7656

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

Merged
merged 1 commit into from
Apr 10, 2025

Conversation

eduardorittner
Copy link
Contributor

When printing unsigned numbers, GNU coreutils makes negative numbers around, so when we try to get a u64 from a BigInt and it's negative, convert it to an i64 and from that to a u64 so it wraps around.

For the test cases I used the same ones as in the original issue #7488

Closes #7488

@eduardorittner
Copy link
Contributor Author

eduardorittner commented Apr 5, 2025

In terms of readability, I could probably change u64::extended_parse("-9223372036854775808") to u64::extended_parse(&format("{}", i64::MIN)), but I'm not sure what to do with u64::extended_parse("-9223372036854775809"), maybe something like &format("{}", i64::MIN as i128 - 1)?

@eduardorittner eduardorittner force-pushed the main branch 2 times, most recently from 85bf1e2 to ccf810a Compare April 5, 2025 02:35
Copy link

github-actions bot commented Apr 5, 2025

GNU testsuite comparison:

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

@drinkcat
Copy link
Contributor

drinkcat commented Apr 5, 2025

In terms of readability, I could probably change u64::extended_parse("-9223372036854775808") to u64::extended_parse(&format("{}", i64::MIN)), but I'm not sure what to do with u64::extended_parse("-9223372036854775809"), maybe something like &format("{}", i64::MIN as i128 - 1)?

I think it's ok to hardcode values in tests (worth adding a comment though).

@eduardorittner eduardorittner force-pushed the main branch 2 times, most recently from 69bea2b to c6a7534 Compare April 6, 2025 14:03
@eduardorittner
Copy link
Contributor Author

Added some more tests cases, checking the overflow value as well.
Changed test name to unsigned_hex_negative_wraparound and moved it above test_overflow.
Changed the overflow logic to use two's complement so now we are able to parse down to -u64::MAX, like GNU.

@eduardorittner
Copy link
Contributor Author

There's a specific CI which is failing because it thinks that fffffffffffffffc is a word

@cakebaker
Copy link
Contributor

@eduardorittner you can suppress the error by adding the following to the top of the file:

// spell-checker:ignore fffffffffffffffc

Copy link

github-actions bot commented Apr 6, 2025

GNU testsuite comparison:

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

@eduardorittner
Copy link
Contributor Author

@eduardorittner you can suppress the error by adding the following to the top of the file:

// spell-checker:ignore fffffffffffffffc

Thanks!

To convert from negative to unsigned with overflow, we get the two's complement
representation of the absolute (unsigned) value.
Copy link

github-actions bot commented Apr 7, 2025

GNU testsuite comparison:

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

@eduardorittner eduardorittner requested a review from drinkcat April 7, 2025 19:29
Copy link
Contributor

@drinkcat drinkcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate this feature (why is C like this??), but in the name of compatibility I'll approve 😄

@sylvestre sylvestre merged commit a89fc48 into uutils:main Apr 10, 2025
86 of 88 checks passed
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.

printf: negative numbers should wrap around when printed as unsigned/hexadecimal (%u/%x)
5 participants