The Wayback Machine - https://web.archive.org/web/20221223070157/https://github.com/python/cpython/pull/27959
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

bpo-28474: Handle unsigned long win32 error codes #27959

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ameily
Copy link
Contributor

@ameily ameily commented Aug 26, 2021

bpo-28474: Handle unsigned long win32 error codes

Windows Error Codes are DWORD values. This PR ensures that error codes larger than LONG_MAX are handled. Previously, an overflow exception was raised for large, but valid, win32 error codes, such as E_POINTER 0x80000005.

https://bugs.python.org/issue28474

@ameily ameily changed the title Handle unsigned long win32 error codes bpo-27484: Handle unsigned long win32 error codes Aug 26, 2021
@ameily ameily changed the title bpo-27484: Handle unsigned long win32 error codes bpo-28474: Handle unsigned long win32 error codes Aug 26, 2021
Copy link
Member

@zooba zooba left a comment

Change looks good, I think we can improve the error messages slightly.

Modules/_ctypes/callproc.c Outdated Show resolved Hide resolved
Modules/_ctypes/callproc.c Outdated Show resolved Hide resolved
if (winerrcode == -1 && PyErr_Occurred())
return -1;

if(winerrcode < LONG_MIN || winerrcode > ULONG_MAX) {
Copy link
Member

@zooba zooba Aug 26, 2021

Choose a reason for hiding this comment

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

Suggested change
if(winerrcode < LONG_MIN || winerrcode > ULONG_MAX) {
if((int)winerrcode != winerrcode) {

Copy link
Contributor Author

@ameily ameily Aug 27, 2021

Choose a reason for hiding this comment

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

From my understanding, this change would break error codes greater than LONG_MAX, which are still valid and this PR is adding support for. For example, this change raises an exception for a valid DWORD error code.

>>> OSError(0, '', None, 0x80000005)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: error code 2147483653 too big for int

Objects/exceptions.c Outdated Show resolved Hide resolved
@ameily ameily requested a review from zooba Sep 1, 2021
@github-actions
Copy link

github-actions bot commented Oct 2, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Oct 2, 2021
@ameily
Copy link
Contributor Author

ameily commented Oct 2, 2021

@zooba I've rebased from upstream/main and have addressed all feedback items, except one that I think it out of scope. Let me know if you need anything else from me.

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Oct 3, 2021
Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Changing to "|L:FormatError" makes sense.
LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants