Skip to content

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

Merged
mdickinson merged 6 commits intopython:mainfrom
sobolevn:issue-45034
Sep 7, 2021
Merged

bpo-45034: Fixes how upper limit is formatted for struct.pack("H", ...)#28178
mdickinson merged 6 commits intopython:mainfrom
sobolevn:issue-45034

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Sep 5, 2021 •
edited
Loading

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 reviewed changes
@mdickinson
Copy link
Member

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 reviewed changes
sobolevn and others added 2 commits September 6, 2021 11:21
@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 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 reviewed changes
Copy link
Member

@mdickinson mdickinson left a comment

Choose a reason for hiding this comment

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

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 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 reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

What about other integer and floating point types?

@mdickinson
Copy link
Member

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 commented Sep 7, 2021

Let's have an explicit cast then 🙂

@mdickinson mdickinson merged commit 8ca6b61 into python:main Sep 7, 2021
This was referenced Oct 13, 2022
Closed
Merged
@stevendaprano stevendaprano mentioned this pull request May 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

+1 more reviewer

@mdickinson mdickinson mdickinson approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sobolevn @mdickinson @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot