Wrong use of `ctypes.GetLastError()` in windows_console.py

[chris-eibl]

Bug report

Bug description:

Everywhere ctypes.GetLastError() is used, even though use_last_error is specified

cpython/Lib/_pyrepl/windows_console.py

Line 617 in e1c09ff

_KERNEL32 = WinDLL("kernel32", use_last_error=True)

This will always return the "swapped" error, as can be seen here

cpython/Modules/_ctypes/callbacks.c

Lines 201 to 222 in e1c09ff

	#ifdef MS_WIN32
	if (flags & FUNCFLAG_USE_LASTERROR) {
	int temp = space[1];
	space[1] = GetLastError();
	SetLastError(temp);
	}
	#endif
	}
	result = PyObject_Vectorcall(callable, args, nargs, NULL);
	if (result == NULL) {
	PyErr_FormatUnraisable("Exception ignored while "
	"calling ctypes callback function %R",
	callable);
	}
	#ifdef MS_WIN32
	if (flags & FUNCFLAG_USE_LASTERROR) {
	int temp = space[1];
	space[1] = GetLastError();
	SetLastError(temp);
	}

or carefully reading the docs (the code was easier to read for me :)

The rule is easy: use_last_error=True implies using ctypes.get_last_error(), which @eryksun has for example explained here: https://discuss.python.org/t/a-ctypes-function-to-list-all-loaded-shared-libraries/36370/10

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

[chris-eibl]

The easiest fix would be to search/replace, but I'd like to take the opportunity to also introduce errcheck callables like

from functools import partial

def errcheck(result, func, args, error_val):
    if result == error_val:
        raise WinError(get_last_error())
    return result

SetConsoleMode.errcheck = partial(errcheck, error_val=False)
WaitForSingleObject.errcheck = partial(errcheck, error_val=WAIT_FAILED)

This will make the code much cleaner and "automatically check everywhere". E.g., currently none of the SetConsoleMode or GetConsoleMode calls is checked - and thus a search/replace would not fix them.

This makes the diff slightly larger but IMHO still qualifies for backport, since 3.13 is affected, too.
And the missing checks are bugs IMHO, so we'd have to touch those code parts, anyway, and will most probably end up with not really less diff lines ...

I will do a PR if the repl maintainer agree with the above (@ambv, @lysnikolaou, @pablogsal).

[zooba]

My preference is still that we ought to be actively migrating from ctypes to a proper native module for these. Performance and reliability will thank us.

Beyond that, I don't have a real preference, though I'd vote for the simpler fix just because it's not worth investing heavily in code that I think should be completely rewritten 😉