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-37705: Improve the implementation of winerror_to_errno() #15623

Merged

Conversation

ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented Aug 30, 2019

winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.

Many thanks go to Eryk Sun for their posts on the BPO issue.

https://bugs.python.org/issue37705

winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.
@mangrisano
Copy link
Contributor

mangrisano commented Sep 1, 2019

/cc @eryksun @pfmoore @zooba

@aeros aeros added the type-feature A feature request or enhancement label Sep 2, 2019
Copy link
Contributor

@aeros aeros left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @ZackerySpytz.

I am very much in favor of using constants to give the winerrors more semantic meaning and grouping the returns. However, there's several of the constant names which don't describe their purpose in a meaningful way. I'm not familiar with our stance on linking to external documentation within code comments, but it seems like it might be helpful to provide a link to https://docs.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2.

@eryksun
Copy link
Contributor

eryksun commented Sep 4, 2019

Stever Downer (@zooba) mentioned adding a dict in Modules/errnomodule.c that exposes this mapping to Python code. I guess it could be called winerror2errno. Maybe we could also add a winerrorcode dict with the symbolic names of mapped errors, and errors of interest. For the latter, I think it would be better if winerror_to_errno were based on a counted or null-terminated table instead of a switch. Each entry would have the symbolic name as a string (e.g. "ERROR_FILE_NOT_FOUND"), the error code, and the mapped errno value. This avoids needing to maintain supported error codes in two places. Otherwise if we map a new error in PC/errmap.h, we'll have to separately add the symbolic name in Modules/errnomodule.c.

For Winsock errors, I think the winerror2errno and winerrorcode dicts would only need the six that are specially mapped to CRT errno values, since all others are already available as errno values -- both as module constants and in the errorcode dict. Given that, should these six Windows errors remain defined as errno values? For example, now that WSAEINTR maps to EINTR, does it make sense to have errno.WSAEINTR == 10004 and errorcode[10004] == 'WSAEINTR'? In any case, I don't think it's a problem to leave them there.

@zooba
Copy link
Member

zooba commented Sep 9, 2019

I'm merging and backporting this, as the additional error codes are general goodness that will improve all active versions.

Adding a new public API should be only for master.

@zooba zooba merged commit 19052a1 into python:master Sep 9, 2019
@miss-islington
Copy link
Contributor

Thanks @ZackerySpytz for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 9, 2019
…H-15623)

winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.
(cherry picked from commit 19052a1)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@bedevere-bot
Copy link

GH-15749 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 9, 2019
…H-15623)

winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.
(cherry picked from commit 19052a1)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@bedevere-bot
Copy link

GH-15750 is a backport of this pull request to the 3.7 branch.

miss-islington added a commit that referenced this pull request Sep 9, 2019
winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.
(cherry picked from commit 19052a1)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
miss-islington added a commit that referenced this pull request Sep 9, 2019
winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.
(cherry picked from commit 19052a1)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
…H-15623)

winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
…H-15623)

winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
…H-15623)

winerror_to_errno() is no longer automatically generated.
Do not rely on the old _dosmapperr() function.
Add ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ.
arhadthedev added a commit to arhadthedev/cpython that referenced this pull request Nov 23, 2021
After pythonGH-15623 deleted `generrmap.c`, a related mak-file stopped
working. The mak contains generrmap-related rules only so it should be
removed altogether.

Further search for `errmap\.mak|generrmap` regex through content
of CPython files shows no dangling reference left.
miss-islington pushed a commit that referenced this pull request Feb 2, 2022
After GH-15623 deleted `generrmap.c`, a related mak-file stopped working. The mak contains generrmap-related rules only so it should be removed altogether.

Further search for `errmap\.mak|generrmap` regex through content of CPython files shows no dangling reference left.

Since generrmap is already effectively removed, this pull request contains no blurp.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants