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.

Sign up for GitHub

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-23578: Show which offset raise error when using struct.pack #291

Closed
wants to merge 7 commits into from
Closed

bpo-23578: Show which offset raise error when using struct.pack #291

wants to merge 7 commits into from

Conversation

louisom
Copy link
Contributor

@louisom louisom commented Feb 25, 2017 •
edited
Loading

Adding PyErr_SetString and PyErr_Format wrapper, with a global offset
variable to handle this.

struct.pack('!h', 0x8FFFF)
Traceback (most recent call last):
  File "tests.py", line 5, in <module>
    struct.pack('!h', 0x8FFFF)
struct.error: Raise at offset 1, 'h' format requires -32768 <= number <= 32767

@rhettinger
Copy link
Contributor

This looks like a reasonable improvement.

@louisom
Copy link
Contributor Author

louisom commented Apr 5, 2017

@rhettinger I'm not sure about the global variable. Is there any way to test if global variable has the defect when using threads?

louisom added 5 commits April 16, 2017 18:56
@louisom
bpo-23578: Show which offset raise error when using struct.pack
03d3318 
Adding PyErr_SetString and PyErr_Format wrapper, with a global offset
variable to handle this.

>>> struct.pack('!h', 0x8FFFF)
Traceback (most recent call last):
  File "tests.py", line 5, in <module>
    struct.pack('!h', 0x8FFFF)
struct.error: Raise at offset 1, 'h' format requires -32768 <= number <= 32767
@louisom
Make error message more specific, and using snprintf for str format
6bf466c
@louisom
Fix typo
50507b2
@louisom
Rearrange exception message
f915929
@louisom
Remove global offset, add pos parameter, and unittest
bb6b6ed
@louisom
Copy link
Contributor Author

louisom commented Apr 16, 2017

@serhiy-storchaka I removed the global offset variable, and add a new parameter pos to corresponding function. Not sure this kind of big change is better than global variable or not.

Could you help me review this? Thanks.

@serhiy-storchaka serhiy-storchaka self-requested a review April 16, 2017 16:45
@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Dec 8, 2018
auvipy
auvipy suggested changes May 31, 2019
View reviewed changes
Copy link

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

plz re base

@csabella
Copy link
Contributor

This change is from an unknown repository and this GitHub account is no longer active. I'm going to close this PR so that another one could be opened to replace it.

@csabella csabella closed this Jan 20, 2020
jaraco pushed a commit that referenced this pull request Dec 2, 2022
@dependabot-preview
Bump sentry-sdk from 0.13.0 to 0.13.1 (#291)
285a92f
@alynn alynn mannequin mentioned this pull request Apr 19, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@auvipy auvipy auvipy requested changes

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees
No one assigned
Labels
awaiting core review type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@louisom @rhettinger @csabella @auvipy @brettcannon @serhiy-storchaka @Mariatta @the-knights-who-say-ni @bedevere-bot