[PATCH 0/1] MR2242: win32u: Do not set last error in wait_message() for zero count.

[Paul Gofman (＠gofman)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54405

Also fixes Assassin Creed hanging on start (for a similar reason: it doesn't expect a spurious -1 last error).

There is already a fix attempt sent by Ivan Chikish in !2067 (https://gitlab.winehq.org/wine/wine/-/merge_requests/2067). But I suppose it may be fixed simpler like in this patch. wait_message() with zero count is only called from NtUserPeekMessage(). I think even if for some reason driver's MsgWaitForMultipleObjectsEx returns a real error code we still don't want to set last error here when called from PeekMessage().

[PATCH 1/1] win32u: Do not set last error in wait_message() for zero count.

[Paul Gofman]

From: Paul Gofman

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54405
---
 dlls/win32u/message.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c
index ed4198991cf..60598bbc86a 100644
--- a/dlls/win32u/message.c
+++ b/dlls/win32u/message.c
@@ -2121,7 +2121,7 @@ static DWORD wait_message( DWORD count, const HANDLE *handles, DWORD timeout, DW
                                                      mask, flags );
     if (HIWORD(ret))  /* is it an error code? */
     {
-        RtlSetLastWin32Error( RtlNtStatusToDosError(ret) );
+        if (count) RtlSetLastWin32Error( RtlNtStatusToDosError(ret) );
         ret = WAIT_FAILED;
     }
     if (ret == WAIT_TIMEOUT && !count && !timeout) NtYieldExecution();

[Zebediah Figura (＠zfigura)]

Wait, how can the wait fail if we're not passing any handles?

[Paul Gofman (＠gofman)]

There is an explanation in the linked MR which I think makes sense. Drivers' (winex11, winemac) MsgWaitForMultipleObjectsEx returns 'count - 1' if process_events() adds some events. Which doesn't work exactly straightforward if count is 0. The original MR suggests changing the drivers' convention around return value (maybe is a bit wrong way, although that should still be possible to do right). It seems to me that it maybe doesn't worth it (as driver's MsgWaitForMultipleObjectsEx convention is ok for a sane case of non-zero events count). So I am suggesting to do it simpler and rather handle this special peekmessage case with 0 handle count.

[Zebediah Figura (＠zfigura)]

I guess if it were up to me I'd adjust the return value convention, but failing that perhaps this change deserves better documentation in the code? It's not at all clear that's why the check is there...

[Paul Gofman (＠gofman)]

Regarding convention, does it really worth it to return some special value or check for 0 count in each driver for this special case? It seems to me it would make things uglier, count - 1 is not a special value, for all the other cases it indicates that the wait succeeded for the last object which is always queue handle. Instead of checking of zero count I could as well (instead) check for ~0 status, not sure if adding a special one for this case will make anything easier?

[Paul Gofman (＠gofman)]

Well, actually, it seems like there is no need to change any convension, we can just check for 0 count in driver and return 0 in that case which probably will be more straightforward (I checked that there is no other usage of driver's MsgWaitForMultipleObjectsEx for which that would introduce ambiguity). I will resend that way.