-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
bpo-32713: Fix tarfile.itn for large/negative float values #5434
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
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
Lib/tarfile.py
Outdated
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.
Wouldn't be better to convert n to int at the begin of the function?
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.
My concern in doing so was that the loss of precision might lead to unwanted results if it is done before the comparisons (e.g. -0.1 becomes 0). That said, it may not matter in any practical case.
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.
This is a good example why this should be done before the comparisons. With this patch itn() produces incorrect results for values between -1 and 0.
>>> tarfile.itn(-0.1)
bytearray(b'\xff\x00\x00\x00\x00\x00\x00\x00')
>>> tarfile.nti(tarfile.itn(-0.1))
-72057594037927936
Please add this case to tests.
Lib/tarfile.py
Outdated
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.
This is a good example why this should be done before the comparisons. With this patch itn() produces incorrect results for values between -1 and 0.
>>> tarfile.itn(-0.1)
bytearray(b'\xff\x00\x00\x00\x00\x00\x00\x00')
>>> tarfile.nti(tarfile.itn(-0.1))
-72057594037927936
Please add this case to tests.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
4c1b40e to
6a7d765
Compare
6a7d765 to
2423c27
Compare
|
Thank you for the review! I have made the requested changes; please review again |
|
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
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.
Please add a period at the end of the sentence and "Patch by yourname."
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.
done 👍
2423c27 to
feac508
Compare
|
Thanks @shin- for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7. |
…-5434) (cherry picked from commit 72d9b2b) Co-authored-by: Joffrey F <[email protected]>
|
GH-5917 is a backport of this pull request to the 3.7 branch. |
|
Sorry, @shin- and @serhiy-storchaka, I could not cleanly backport this to |
|
GH-5918 is a backport of this pull request to the 3.6 branch. |
…-5434) (cherry picked from commit 72d9b2b) Co-authored-by: Joffrey F <[email protected]>
(cherry picked from commit 72d9b2b) Co-authored-by: Joffrey F <[email protected]>
(cherry picked from commit 72d9b2b) Co-authored-by: Joffrey F <[email protected]>
This PR changes the behavior of
tarfile.itnto cast the input argumentnto an integer for bitwise operations.https://bugs.python.org/issue32713