Theoretically, this allows debuggers to tell them apart. https://devblogs.microsoft.com/oldnewthing/20110429-00/?p=10803
But in practice, this is more about cleanliness than anything else; I'm not aware of anyone running SetLastError-aware debuggers in Wine. Feel free to reject if this is a waste of time.
The changeable SetLastError calls were found by this script:
```python #!/usr/bin/env python3 import os import re
for (dirpath, dirnames, filenames) in os.walk("."): for fn in filenames: if not fn.endswith(".c"): continue text = open(dirpath+"/"+fn,"rt").read() text = text.replace("RtlSetLastWin32Error","SetLastError") text = text.replace("RtlGetLastWin32Error","GetLastError") if "SetLastError" not in text: continue varname = None for line in text.split("\n"): if "GetLastError" in line: if m := re.search(r"([A-Za-z0-9_]+) *= *GetLastError", line): prevline = line varname = m[1] if line == "}": varname = None if varname is not None and "SetLastError" in line: if m := re.search(r"SetLastError( *([A-Za-z0-9_]+) *)", line): if m[1] == varname: print(dirpath+"/"+fn, prevline, line) ```
The script also flags https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/mpr/wnet.c#L2811, which is half set and half restore; I chose to left it unchanged.
Idea from https://gitlab.winehq.org/wine/wine/-/merge_requests/5020/diffs#6c845445f68f...
From: Alfred Agrell floating@muncher.se
--- dlls/advapi32/tests/security.c | 2 +- dlls/comctl32/tests/edit.c | 2 +- dlls/crypt32/protectdata.c | 2 +- dlls/dbghelp/tests/dbghelp.c | 2 +- dlls/mciwave/mciwave.c | 2 +- dlls/mspatcha/pa19.c | 4 ++-- dlls/msvcrt/thread.c | 2 +- dlls/riched20/tests/editor.c | 2 +- dlls/riched20/tests/richole.c | 2 +- dlls/setupapi/misc.c | 2 +- dlls/user32/clipboard.c | 2 +- dlls/user32/tests/clipboard.c | 6 +++--- dlls/user32/tests/edit.c | 2 +- dlls/user32/tests/msg.c | 2 +- dlls/winecrt0/debug.c | 2 +- 15 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index 2840f6bd75a..0ce1e515387 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -133,7 +133,7 @@ static const char* debugstr_sid(PSID sid) LocalFree(sidstr); } /* Restore the last error in case ConvertSidToStringSidA() modified it */ - SetLastError(le); + RestoreLastError(le); return res; }
diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index bce0215ef19..cab5c918b67 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -94,7 +94,7 @@ static BOOL open_clipboard(HWND hwnd) */ GetClassNameA(clipwnd, classname, ARRAY_SIZE(classname)); trace("%p (%s) opened the clipboard\n", clipwnd, classname); - SetLastError(le); + RestoreLastError(le); return ret; } Sleep(15); diff --git a/dlls/crypt32/protectdata.c b/dlls/crypt32/protectdata.c index f7d9f439872..3a5997ea1a8 100644 --- a/dlls/crypt32/protectdata.c +++ b/dlls/crypt32/protectdata.c @@ -729,7 +729,7 @@ BOOL load_encryption_key(HCRYPTPROV hProv, DWORD key_len, const DATA_BLOB *salt, szUsername[0]='\0'; GetUserNameA( szUsername, &dwUsernameLen ); } - SetLastError(dwError); + RestoreLastError(dwError);
/* salt the hash with: * - the user id diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index d1c2f219533..7c79609def1 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -748,7 +748,7 @@ static enum process_kind get_process_kind(HANDLE process) { DWORD gle = GetLastError(); enum process_kind pcskind = get_process_kind_internal(process); - SetLastError(gle); + RestoreLastError(gle); return pcskind; }
diff --git a/dlls/mciwave/mciwave.c b/dlls/mciwave/mciwave.c index f2af38b8762..e373d1c093e 100644 --- a/dlls/mciwave/mciwave.c +++ b/dlls/mciwave/mciwave.c @@ -1445,7 +1445,7 @@ static DWORD WAVE_mciSave(MCIDEVICEID wDevID, DWORD dwFlags, LPMCI_SAVE_PARMSW l */ tmpRet = GetLastError(); DeleteFileW (lpParms->lpfilename); - SetLastError(tmpRet); + RestoreLastError(tmpRet);
/* FIXME: Open file.wav; Save; must not rename the original file. * Nor must Save a.wav; Save b.wav rename a. */ diff --git a/dlls/mspatcha/pa19.c b/dlls/mspatcha/pa19.c index 4b21b199ed8..93abf45366f 100644 --- a/dlls/mspatcha/pa19.c +++ b/dlls/mspatcha/pa19.c @@ -903,7 +903,7 @@ close_old_map: close_patch_map: CloseHandle(patch_map);
- SetLastError(err); + RestoreLastError(err);
return res; } @@ -965,7 +965,7 @@ close_patch_file: CloseHandle(patch_hndl);
/* set last error even on success as per windows */ - SetLastError(err); + RestoreLastError(err);
return res; } diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 6aad9a22d47..1462340303d 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -61,7 +61,7 @@ thread_data_t *CDECL msvcrt_get_thread_data(void) ptr->module = NULL; #endif } - SetLastError( err ); + RestoreLastError( err ); return ptr; }
diff --git a/dlls/riched20/tests/editor.c b/dlls/riched20/tests/editor.c index f8abb7a3e65..49662207d69 100644 --- a/dlls/riched20/tests/editor.c +++ b/dlls/riched20/tests/editor.c @@ -4835,7 +4835,7 @@ static BOOL open_clipboard_(int line, HWND hwnd) */ GetClassNameA(clipwnd, classname, ARRAY_SIZE(classname)); trace_(__FILE__, line)("%p (%s) opened the clipboard\n", clipwnd, classname); - SetLastError(le); + RestoreLastError(le); return ret; } Sleep(15); diff --git a/dlls/riched20/tests/richole.c b/dlls/riched20/tests/richole.c index fb5177b83e9..c5ace5f589c 100644 --- a/dlls/riched20/tests/richole.c +++ b/dlls/riched20/tests/richole.c @@ -4955,7 +4955,7 @@ static BOOL open_clipboard(HWND hwnd) */ GetClassNameA(clipwnd, classname, ARRAY_SIZE(classname)); trace("%p (%s) opened the clipboard\n", clipwnd, classname); - SetLastError(le); + RestoreLastError(le); return ret; } Sleep(15); diff --git a/dlls/setupapi/misc.c b/dlls/setupapi/misc.c index b1083fca665..9d03787c385 100644 --- a/dlls/setupapi/misc.c +++ b/dlls/setupapi/misc.c @@ -456,7 +456,7 @@ BOOL WINAPI FileExists(LPCWSTR lpFileName, LPWIN32_FIND_DATAW lpFileFindData) { dwError = GetLastError(); SetErrorMode(uErrorMode); - SetLastError(dwError); + RestoreLastError(dwError); return FALSE; }
diff --git a/dlls/user32/clipboard.c b/dlls/user32/clipboard.c index 72c7720dc17..d6cb3e88dae 100644 --- a/dlls/user32/clipboard.c +++ b/dlls/user32/clipboard.c @@ -56,7 +56,7 @@ static const char *debugstr_format( UINT id ) WCHAR buffer[256]; DWORD le = GetLastError(); BOOL r = NtUserGetClipboardFormatName( id, buffer, 256 ); - SetLastError(le); + RestoreLastError(le);
if (r) return wine_dbg_sprintf( "%04x %s", id, debugstr_w(buffer) ); diff --git a/dlls/user32/tests/clipboard.c b/dlls/user32/tests/clipboard.c index 87df06b6621..90d9f301721 100644 --- a/dlls/user32/tests/clipboard.c +++ b/dlls/user32/tests/clipboard.c @@ -57,7 +57,7 @@ static BOOL open_clipboard_(int line, HWND hwnd) */ GetClassNameA(clipwnd, classname, ARRAY_SIZE(classname)); trace_(__FILE__, line)("%p (%s) opened the clipboard\n", clipwnd, classname); - SetLastError(le); + RestoreLastError(le); return ret; } Sleep(15); @@ -80,11 +80,11 @@ static BOOL has_no_open_wnd_(int line) /* See open_clipboard() */ GetClassNameA(clipwnd, classname, ARRAY_SIZE(classname)); trace_(__FILE__, line)("%p (%s) opened the clipboard\n", clipwnd, classname); - SetLastError(le); + RestoreLastError(le); return FALSE; } Sleep(15); - SetLastError(le); + RestoreLastError(le); } }
diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index bbde9b16616..aff286907c1 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -54,7 +54,7 @@ static BOOL open_clipboard(HWND hwnd) */ GetClassNameA(clipwnd, classname, ARRAY_SIZE(classname)); trace("%p (%s) opened the clipboard\n", clipwnd, classname); - SetLastError(le); + RestoreLastError(le); return ret; } Sleep(15); diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 3414dda6e5a..406d714e0e5 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -17965,7 +17965,7 @@ static BOOL open_clipboard_(int line, HWND hwnd) */ GetClassNameA(clipwnd, classname, ARRAY_SIZE(classname)); trace_(__FILE__, line)("%p (%s) opened the clipboard\n", clipwnd, classname); - SetLastError(le); + RestoreLastError(le); return ret; } Sleep(15); diff --git a/dlls/winecrt0/debug.c b/dlls/winecrt0/debug.c index 2ac4505fb85..e150aa20289 100644 --- a/dlls/winecrt0/debug.c +++ b/dlls/winecrt0/debug.c @@ -54,7 +54,7 @@ static void load_func( void **func, const char *name, void *def ) HMODULE module = GetModuleHandleW( L"ntdll.dll" ); void *proc = GetProcAddress( module, name ); InterlockedExchangePointer( func, proc ? proc : def ); - SetLastError( err ); + RestoreLastError( err ); } } #define LOAD_FUNC(name) load_func( (void **)&p ## name, #name, fallback ## name )
From: Alfred Agrell floating@muncher.se
This allows debuggers to tell them apart --- dlls/ntdll/error.c | 18 +++++++++++++++++- dlls/ntdll/ntdll.spec | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/error.c b/dlls/ntdll/error.c index 5412b19907c..6b914f226e5 100644 --- a/dlls/ntdll/error.c +++ b/dlls/ntdll/error.c @@ -109,7 +109,6 @@ DWORD WINAPI RtlGetLastWin32Error(void)
/*********************************************************************** * RtlSetLastWin32Error (NTDLL.@) - * RtlRestoreLastWin32Error (NTDLL.@) * * Set the per-thread error value. * @@ -124,6 +123,23 @@ void WINAPI RtlSetLastWin32Error( DWORD err ) NtCurrentTeb()->LastErrorValue = err; }
+/*********************************************************************** + * RtlRestoreLastWin32Error (NTDLL.@) + * + * Set the per-thread error value. + * Identical to the above; duplicated to allow debugger breakpoints to tell them apart. + * + * PARAMS + * err [I] The new error value to set + * + * RETURNS + * Nothing. + */ +void WINAPI RtlRestoreLastWin32Error( DWORD err ) +{ + NtCurrentTeb()->LastErrorValue = err; +} + /*********************************************************************** * RtlSetLastWin32ErrorAndNtStatusFromNtStatus (NTDLL.@) * diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 64fdb6fdb24..b3650d7113d 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -968,7 +968,7 @@ @ stdcall RtlRemoveVectoredExceptionHandler(ptr) @ stdcall RtlResetRtlTranslations(ptr) @ cdecl RtlRestoreContext(ptr ptr) -@ stdcall RtlRestoreLastWin32Error(long) RtlSetLastWin32Error +@ stdcall RtlRestoreLastWin32Error(long) @ stub RtlRevertMemoryStream @ stub RtlRunDecodeUnicodeString @ stub RtlRunEncodeUnicodeString
From: Alfred Agrell floating@muncher.se
--- dlls/win32u/clipboard.c | 2 +- dlls/win32u/spy.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/dlls/win32u/clipboard.c b/dlls/win32u/clipboard.c index 6cf484a56ca..8b8c5f0ea2d 100644 --- a/dlls/win32u/clipboard.c +++ b/dlls/win32u/clipboard.c @@ -58,7 +58,7 @@ static const char *debugstr_format( UINT id ) WCHAR buffer[256]; DWORD le = RtlGetLastWin32Error(); BOOL r = NtUserGetClipboardFormatName( id, buffer, ARRAYSIZE(buffer) ); - RtlSetLastWin32Error(le); + RtlRestoreLastWin32Error(le);
if (r) return wine_dbg_sprintf( "%04x %s", id, debugstr_w(buffer) ); diff --git a/dlls/win32u/spy.c b/dlls/win32u/spy.c index 20a57803cb6..a0422d69434 100644 --- a/dlls/win32u/spy.c +++ b/dlls/win32u/spy.c @@ -2220,7 +2220,7 @@ const char *debugstr_msg_name( UINT msg, HWND hWnd ) ext_sp_e.wParam = 0; ext_sp_e.wnd_class[0] = 0; SPY_GetMsgStuff(&ext_sp_e); - RtlSetLastWin32Error( save_error ); + RtlRestoreLastWin32Error( save_error ); return wine_dbg_sprintf("%s", ext_sp_e.msg_name); }
@@ -2505,7 +2505,7 @@ static void SPY_DumpStructure(const SPY_INSTANCE *sp_e, BOOL enter) /* save and restore error code over the next call */ save_error = RtlGetLastWin32Error(); NtUserGetClassName( pnmh->hwndFrom, FALSE, &str ); - RtlSetLastWin32Error(save_error); + RtlRestoreLastWin32Error(save_error); if (wcscmp(TOOLBARCLASSNAMEW, from_class) == 0) dumplen = sizeof(NMTBCUSTOMDRAW)-sizeof(NMHDR); } else if ( pnmh->code >= HDN_ENDDRAG @@ -2639,7 +2639,7 @@ void spy_enter_message( INT iFlag, HWND hWnd, UINT msg, WPARAM wParam, LPARAM lP break; } set_indent_level( indent + SPY_INDENT_UNIT ); - RtlSetLastWin32Error( save_error ); + RtlRestoreLastWin32Error( save_error ); }
@@ -2684,5 +2684,5 @@ void spy_exit_message( INT iFlag, HWND hWnd, UINT msg, LRESULT lReturn, SPY_DumpStructure(&sp_e, FALSE); break; } - RtlSetLastWin32Error( save_error ); + RtlRestoreLastWin32Error( save_error ); }
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=142835
Your paranoid android.
=== build (build log) ===
/home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' collect2: error: ld returned 1 exit status collect2: error: ld returned 1 exit status /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' collect2: error: ld returned 1 exit status /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/winecrt0/debug.c:57: undefined reference to `RestoreLastError' collect2: error: ld returned 1 exit status Task: The exe32 Wine build failed
=== debian11 (build log) ===
/home/winetest/tools/testbot/var/wine-win32/../wine/dlls/win32u/clipboard.c:61: undefined reference to `RtlRestoreLastWin32Error' /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/win32u/spy.c:2508: undefined reference to `RtlRestoreLastWin32Error' /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/win32u/spy.c:2223: undefined reference to `RtlRestoreLastWin32Error' /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/win32u/spy.c:2642: undefined reference to `RtlRestoreLastWin32Error' /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/win32u/spy.c:2687: undefined reference to `RtlRestoreLastWin32Error' collect2: error: ld returned 1 exit status Task: The win32 Wine build failed
=== debian11b (build log) ===
/home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/win32u/clipboard.c:61: undefined reference to `RtlRestoreLastWin32Error' /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/win32u/spy.c:2508: undefined reference to `RtlRestoreLastWin32Error' /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/win32u/spy.c:2223: undefined reference to `RtlRestoreLastWin32Error' /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/win32u/spy.c:2642: undefined reference to `RtlRestoreLastWin32Error' /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/win32u/spy.c:2687: undefined reference to `RtlRestoreLastWin32Error' collect2: error: ld returned 1 exit status Task: The wow64 Wine build failed
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)
-
Marvin