classification
Title: winerror_to_errno implementation
Type: enhancement Stage: resolved
Components: Interpreter Core, Windows Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, eryksun, miss-islington, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2019-07-29 13:41 by eryksun, last changed 2019-09-09 12:17 by steve.dower. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15623 merged ZackerySpytz, 2019-08-30 21:19
PR 15749 merged miss-islington, 2019-09-09 09:35
PR 15750 merged miss-islington, 2019-09-09 09:35
Messages (9)
msg348661 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-07-29 13:41
OSError and _Py_fstat_noraise rely on winerror_to_errno() in PC/errmap.h in order to translate a Windows system error code into a POSIX errno value. PC/errmap.h is supposed to be generated by PC/generrmap.c, which is based on the old CRT's _dosmapperr() function. However, this function isn't implemented in the Universal CRT, and ucrt's corresponding function, __acrt_errno_from_os_error, isn't exported for public use.

If generrmap.c is effectively dead, then it's no longer possible to add custom mappings there, as was done for issue 12802 (ERROR_DIRECTORY -> ENOTDIR) and issue 13063 (ERROR_NO_DATA -> EPIPE). Also, errmap.h hasn't been regenerated in 8 years, and since then the CRT added a new mapped value: ERROR_NO_UNICODE_TRANSLATION (1113) -> EILSEQ. 

Unless someone can suggest a way to continue automatically generating errmap.h via generrmap.c, then I think winerror_to_errno should be manually implemented in a more readable and maintainable way, and updated from the ucrt source (ucrt\misc\errno.cpp) with each major Python release. The implementation could use a switch statement like it currently does, but with named error codes, grouped by result. For example:

    int
    winerror_to_errno(int winerror)
    {
        switch(winerror) {
        case ERROR_FILE_NOT_FOUND:            //    2
        case ERROR_PATH_NOT_FOUND:            //    3
        case ERROR_INVALID_DRIVE:             //   15
        case ERROR_NO_MORE_FILES:             //   18
        case ERROR_BAD_NETPATH:               //   53
        case ERROR_BAD_NET_NAME:              //   67
        case ERROR_BAD_PATHNAME:              //  161
        case ERROR_FILENAME_EXCED_RANGE:      //  206
            return ENOENT;

        case ERROR_BAD_ENVIRONMENT:           //   10
            return E2BIG;

        case ERROR_BAD_FORMAT:                //   11
        case ERROR_INVALID_STARTING_CODESEG:  //  188
        case ERROR_INVALID_STACKSEG:          //  189
        case ERROR_INVALID_MODULETYPE:        //  190
        case ERROR_INVALID_EXE_SIGNATURE:     //  191
        case ERROR_EXE_MARKED_INVALID:        //  192
        case ERROR_BAD_EXE_FORMAT:            //  193
        case ERROR_ITERATED_DATA_EXCEEDS_64k: //  194
        case ERROR_INVALID_MINALLOCSIZE:      //  195
        case ERROR_DYNLINK_FROM_INVALID_RING: //  196
        case ERROR_IOPL_NOT_ENABLED:          //  197
        case ERROR_INVALID_SEGDPL:            //  198
        case ERROR_AUTODATASEG_EXCEEDS_64k:   //  199
        case ERROR_RING2SEG_MUST_BE_MOVABLE:  //  200
        case ERROR_RELOC_CHAIN_XEEDS_SEGLIM:  //  201
        case ERROR_INFLOOP_IN_RELOC_CHAIN:    //  202
            return ENOEXEC;

        case ERROR_INVALID_HANDLE:            //    6
        case ERROR_INVALID_TARGET_HANDLE:     //  114
        case ERROR_DIRECT_ACCESS_HANDLE:      //  130
            return EBADF;

        case ERROR_WAIT_NO_CHILDREN:          //  128
        case ERROR_CHILD_NOT_COMPLETE:        //  129
            return ECHILD;

        case ERROR_NO_PROC_SLOTS:             //   89
        case ERROR_MAX_THRDS_REACHED:         //  164
        case ERROR_NESTING_NOT_ALLOWED:       //  215
            return EAGAIN;

        case ERROR_ARENA_TRASHED:             //    7
        case ERROR_NOT_ENOUGH_MEMORY:         //    8
        case ERROR_INVALID_BLOCK:             //    9
        case ERROR_NOT_ENOUGH_QUOTA:          // 1816
            return ENOMEM;

        case ERROR_ACCESS_DENIED:             //    5
        case ERROR_CURRENT_DIRECTORY:         //   16
        case ERROR_WRITE_PROTECT:             //   19
        case ERROR_BAD_UNIT:                  //   20
        case ERROR_NOT_READY:                 //   21
        case ERROR_BAD_COMMAND:               //   22
        case ERROR_CRC:                       //   23
        case ERROR_BAD_LENGTH:                //   24
        case ERROR_SEEK:                      //   25
        case ERROR_NOT_DOS_DISK:              //   26
        case ERROR_SECTOR_NOT_FOUND:          //   27
        case ERROR_OUT_OF_PAPER:              //   28
        case ERROR_WRITE_FAULT:               //   29
        case ERROR_READ_FAULT:                //   30
        case ERROR_GEN_FAILURE:               //   31
        case ERROR_SHARING_VIOLATION:         //   32
        case ERROR_LOCK_VIOLATION:            //   33
        case ERROR_WRONG_DISK:                //   34
        case ERROR_SHARING_BUFFER_EXCEEDED:   //   36
        case ERROR_NETWORK_ACCESS_DENIED:     //   65
        case ERROR_CANNOT_MAKE:               //   82
        case ERROR_FAIL_I24:                  //   83
        case ERROR_DRIVE_LOCKED:              //  108
        case ERROR_SEEK_ON_DEVICE:            //  132
        case ERROR_NOT_LOCKED:                //  158
        case ERROR_LOCK_FAILED:               //  167
        case 35:                              //   35 [undefined]
            return EACCES;

        case ERROR_FILE_EXISTS:               //   80
        case ERROR_ALREADY_EXISTS:            //  183
            return EEXIST;

        case ERROR_NOT_SAME_DEVICE:           //   17
            return EXDEV;

        case ERROR_DIRECTORY:                 //  267 [bpo-12802]
            return ENOTDIR;

        case ERROR_TOO_MANY_OPEN_FILES:       //    4
            return EMFILE;

        case ERROR_DISK_FULL:                 //  112
            return ENOSPC;

        case ERROR_BROKEN_PIPE:               //  109
        case ERROR_NO_DATA:                   //  232 [bpo-13063]
            return EPIPE;

        case ERROR_DIR_NOT_EMPTY:             //  145
            return ENOTEMPTY;

        case ERROR_NO_UNICODE_TRANSLATION:    // 1113
            return EILSEQ;

        case ERROR_INVALID_FUNCTION:          //    1
        case ERROR_INVALID_ACCESS:            //   12
        case ERROR_INVALID_DATA:              //   13
        case ERROR_INVALID_PARAMETER:         //   87
        case ERROR_NEGATIVE_SEEK:             //  131
        default:
            return EINVAL;
        }
    }
msg348768 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-07-30 18:35
Special casing of Winsock error codes should be handled in winerror_to_errno rather than in oserror_parse_args, so it's consistently applied everywhere. And it should use the proper range for Winsock, 10000-11999 -- not 10000-2147483647. Codes above the Winsock range should map to EINVAL. It's not a non-issue since this range has the following blocks allocated:

    IPSEC     13000-13999
    SXS       14000-14999
    EVT       15000-15079
    EC        15080-15099
    MUI       15100-15199
    MCA       15200-15249
    SYSPART   15250-15299
    VORTEX    15300-15320
    GPIO      15321-15340
    RUNLEVEL  15400-15500
    COMTASK   15501-15510
    APPX      15600-15699
    APPMODEL  15700-15720
    APPXSTATE 15800-15840
    API       15841-15860
    STORE     15861-15880

Another thing winerror_to_errno should do is unwrap HRESULT codes for Win32 errors (0x80070000-0x8007FFFF). In other words, 0x80070005 is effectively the same as ERROR_ACCESS_DENIED (5). It should be mapped to EACCES and raise PermissionError.

For example:

    int
    winerror_to_errno(int winerror)
    {
        /* Unwrap FACILITY_WIN32 HRESULT errors. */
        if ((winerror & 0xFFFF0000) == 0x80070000) {
            winerror &= 0x0000FFFF;
        }

        /* Winsock error codes (10000-11999) are errno values. */
        if (winerror >= 10000 && winerror < 12000) {
            return winerror;
        }

        switch(winerror) {
        
        /* ... */
        
        }
    }
msg350552 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-08-26 17:49
While I'm inclined to think it's okay to find the CRT sources (e.g. "C:\Program Files (x86)\Windows Kits\10\Source\10.0.18362.0\ucrt\misc\errno.cpp") and extract the table from there as part of build, the problem I have is that it is woefully incomplete.

I'd much rather have a manually managed table. It'd also be nice to expose it through the errno module somehow, probably just as a dict.

It also seems like at least some of the WSA* constants (e.g. WSAEBADF==10009) are the equivalent errno plus 10000 (e.g.EBADF==9). Should we be mapping those back to the errno value?
msg350585 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-08-27 00:48
> It also seems like at least some of the WSA* constants (e.g. 
> WSAEBADF==10009) are the equivalent errno plus 10000 
> (e.g.EBADF==9). Should we be mapping those back to the 
> errno value?

Mapping WSAEINTR (10004) -> EINTR (4) and WSAEACCES (10013) -> EACCES (13) would allow special casing them as InterruptedError and PermissionError. This is different from the aliases such as EWOULDBLOCK = WSAEWOULDBLOCK, but it should be fine since the six affected values are singled out in WinSock2.h as C errno values. 

So the Winsock section becomes a switch:

        /* Winsock error codes (10000-11999) are errno values. */
        if (winerror >= 10000 && winerror < 12000) {
            switch(winerror) {
            case WSAEINTR:
            case WSAEBADF:
            case WSAEACCES:
            case WSAEFAULT:
            case WSAEINVAL:
            case WSAEMFILE:
                /* Winsock definitions of errno values. See WinSock2.h */
                return winerror - 10000;
            default:
                return winerror;
            }
        }
msg350889 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2019-08-30 21:21
I have created a pull request for this issue. Please take a look.
msg351378 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-09-09 09:33
I'm going to merge and backport this all the way to 3.7. Adding a publicly accessible dict for use in Python code should be 3.9 only.
msg351380 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-09-09 09:35
New changeset 19052a11314e7be7ba003fd6cdbb5400a5d77d96 by Steve Dower (Zackery Spytz) in branch 'master':
bpo-37705: Improve the implementation of winerror_to_errno() (GH-15623)
https://github.com/python/cpython/commit/19052a11314e7be7ba003fd6cdbb5400a5d77d96
msg351415 - (view) Author: miss-islington (miss-islington) Date: 2019-09-09 10:37
New changeset 68e401fa0b1a3407bce395ad893535f65107ee6e by Miss Islington (bot) in branch '3.8':
bpo-37705: Improve the implementation of winerror_to_errno() (GH-15623)
https://github.com/python/cpython/commit/68e401fa0b1a3407bce395ad893535f65107ee6e
msg351417 - (view) Author: miss-islington (miss-islington) Date: 2019-09-09 10:39
New changeset c25759187829d5732eea73f52a269c88aba28371 by Miss Islington (bot) in branch '3.7':
bpo-37705: Improve the implementation of winerror_to_errno() (GH-15623)
https://github.com/python/cpython/commit/c25759187829d5732eea73f52a269c88aba28371
History
Date User Action Args
2019-09-09 12:17:30steve.dowersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-09-09 10:39:25miss-islingtonsetmessages: + msg351417
2019-09-09 10:37:58miss-islingtonsetnosy: + miss-islington
messages: + msg351415
2019-09-09 09:35:26miss-islingtonsetpull_requests: + pull_request15404
2019-09-09 09:35:20miss-islingtonsetpull_requests: + pull_request15403
2019-09-09 09:35:11steve.dowersetmessages: + msg351380
2019-09-09 09:33:54steve.dowersetmessages: + msg351378
versions: + Python 3.7, Python 3.8
2019-08-30 21:21:16ZackerySpytzsetnosy: + ZackerySpytz

messages: + msg350889
versions: - Python 3.8
2019-08-30 21:19:18ZackerySpytzsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request15291
2019-08-27 00:48:33eryksunsetmessages: + msg350585
2019-08-26 17:49:40steve.dowersetmessages: + msg350552
2019-07-30 18:35:10eryksunsetmessages: + msg348768
2019-07-29 13:41:00eryksuncreate