Skip to content

gh-72660: Handle unsigned long win32 error codes#27959

Open
ameily wants to merge 7 commits intopython:mainfrom
ameily:fix-issue-28474
Open

gh-72660: Handle unsigned long win32 error codes#27959
ameily wants to merge 7 commits intopython:mainfrom
ameily:fix-issue-28474

Conversation

@ameily
Copy link
Copy Markdown
Contributor

@ameily ameily commented Aug 26, 2021 •
edited by bedevere-bot
Loading

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
zooba
zooba reviewed Aug 26, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@zooba zooba left a comment

Choose a reason for hiding this comment

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

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

@ameily ameily requested a review from zooba September 1, 2021 22:24
@github-actions
Copy link
Copy Markdown

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
Copy Markdown
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
MaxwellDupre
MaxwellDupre approved these changes Jul 23, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

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

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

@arhadthedev arhadthedev changed the title bpo-28474: Handle unsigned long win32 error codes gh-72660: Handle unsigned long win32 error codes Apr 29, 2023
@arhadthedev arhadthedev modified the milestone: n100095 Apr 30, 2023
@arhadthedev arhadthedev requested review from a team and iritkatriel as code owners April 30, 2023 14:47
@zooba
Copy link
Copy Markdown
Member

zooba commented May 2, 2023

The failed tests look like winerror_to_errno isn't handling error code with the top bit set properly, but I suspect it's due to the conversions we've added.

I'm also a little concerned about changing values in OSError.winerror for users. Not sure whether our tests cover that, but that seems like an annoying change that I'd rather not make.1

Footnotes

  1. Won't affect my own code, because I always ex.winerror & 0xFFFFFFFF before comparing, but I don't believe that's a universal pattern. ↩

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot bot commented Apr 18, 2025

The following commit authors need to sign the Contributor License Agreement:

CLA signed

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

Reviewers

@zooba zooba Awaiting requested review from zooba

@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner

1 more reviewer

@MaxwellDupre MaxwellDupre MaxwellDupre approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting core review OS-windows topic-ctypes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ameily @zooba @MaxwellDupre @arhadthedev @the-knights-who-say-ni @ezio-melotti @bedevere-bot