Skip to content
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

Jump to bottom

bpo-45034: Fixes how upper limit is formatted for struct.pack("H", ...) #28178

Merged
merged 6 commits into from Sep 7, 2021
Merged

bpo-45034: Fixes how upper limit is formatted for struct.pack("H", ...) #28178

merged 6 commits into from Sep 7, 2021

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Sep 5, 2021 •
edited

https://bugs.python.org/issue45034

It was:

struct.error: ushort format requires 0 <= number <= (0x7fff * 2 + 1)

It is now:

struct.error: ushort format requires 0 <= number <= 65535
mdickinson
mdickinson reviewed Sep 6, 2021
View changes
Modules/_struct.c Outdated Show resolved Hide resolved
@mdickinson
Copy link
Member

@mdickinson mdickinson commented Sep 6, 2021

Thanks for the PR. I think we should extend the fix to np_short, too: those uses of Py_STRINGIFY never really made much sense.

mdickinson
mdickinson reviewed Sep 6, 2021
View changes
Modules/_struct.c Outdated Show resolved Hide resolved
sobolevn and others added 2 commits Sep 6, 2021
@sobolevn @mdickinson
Update Modules/_struct.c
fba681c 
Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
@sobolevn @mdickinson
Update Modules/_struct.c
9f285bb 
Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
@sobolevn
Copy link
Member Author

@sobolevn sobolevn commented Sep 6, 2021

I think we should extend the fix to np_short, too: those uses of Py_STRINGIFY never really made much sense.

Will do! Thanks a lot for your help 👍

mdickinson
mdickinson approved these changes Sep 7, 2021
View changes
Copy link
Member

@mdickinson mdickinson left a comment

LGTM; thank you! Do you want to add your name to the list in Misc/ACKS? (It's entirely optional.)

@sobolevn
Copy link
Member Author

@sobolevn sobolevn commented Sep 7, 2021

Do you want to add your name to the list in Misc/ACKS?

Why not? 😄 Added!

Thanks a lot for your help! 👍

serhiy-storchaka
serhiy-storchaka reviewed Sep 7, 2021
View changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

What about other integer and floating point types?

Modules/_struct.c Outdated Show resolved Hide resolved
@mdickinson
Copy link
Member

@mdickinson mdickinson commented Sep 7, 2021

What about other integer and floating point types?

I think we can leave broader changes for a separate PR, if anyone feels inclined; this PR addresses the immediate issue that @stevendaprano was seeing, which was due to inappropriate uses of Py_STRINGIFY.

@sobolevn @serhiy-storchaka
Update Modules/_struct.c
c57e30c 
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@sobolevn
Copy link
Member Author

@sobolevn sobolevn commented Sep 7, 2021

Let's have an explicit cast then 🙂

@mdickinson mdickinson merged commit 8ca6b61 into python:main Sep 7, 2021
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdickinson mdickinson

@serhiy-storchaka serhiy-storchaka

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
5 participants
@sobolevn @mdickinson @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot