[PATCH v2 1/2] user32: Return -1 from peek_message() on error.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/user32/message.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/dlls/user32/message.c b/dlls/user32/message.c index 860ceefc38d..4231225ae61 100644 --- a/dlls/user32/message.c +++ b/dlls/user32/message.c @@ -2743,10 +2743,11 @@ static inline void call_sendmsg_callback( SENDASYNCPROC callback, HWND hwnd, UIN /*********************************************************************** * peek_message * - * Peek for a message matching the given parameters. Return FALSE if none available. + * Peek for a message matching the given parameters. Return 0 if none are + * available; -1 on error. * All pending sent messages are processed before returning. */ -static BOOL peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags, UINT changed_mask ) +static int peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags, UINT changed_mask ) { LRESULT result; struct user_thread_info *thread_info = get_user_thread_info(); @@ -2756,7 +2757,7 @@ static BOOL peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags void *buffer; size_t buffer_size = 256; - if (!(buffer = HeapAlloc( GetProcessHeap(), 0, buffer_size ))) return FALSE; + if (!(buffer = HeapAlloc( GetProcessHeap(), 0, buffer_size ))) return -1; if (!first && !last) last = ~0; if (hwnd == HWND_BROADCAST) hwnd = HWND_TOPMOST; @@ -2804,9 +2805,14 @@ static BOOL peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags { thread_info->wake_mask = changed_mask & (QS_SENDMESSAGE | QS_SMRESULT); thread_info->changed_mask = changed_mask; + return 0; } - if (res != STATUS_BUFFER_OVERFLOW) return FALSE; - if (!(buffer = HeapAlloc( GetProcessHeap(), 0, buffer_size ))) return FALSE; + if (res != STATUS_BUFFER_OVERFLOW) + { + SetLastError( RtlNtStatusToDosError(res) ); + return -1; + } + if (!(buffer = HeapAlloc( GetProcessHeap(), 0, buffer_size ))) return -1; continue; } @@ -2927,7 +2933,7 @@ static BOOL peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags thread_info->GetMessageExtraInfoVal = msg_data->hardware.info; HeapFree( GetProcessHeap(), 0, buffer ); HOOK_CallHooks( WH_GETMESSAGE, HC_ACTION, flags & PM_REMOVE, (LPARAM)msg, TRUE ); - return TRUE; + return 1; } continue; case MSG_POSTED: @@ -2941,7 +2947,7 @@ static BOOL peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags if (first == info.msg.message && last == info.msg.message) { HeapFree( GetProcessHeap(), 0, buffer ); - return FALSE; + return 0; } } else @@ -2963,7 +2969,7 @@ static BOOL peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags thread_info->msg_source = msg_source_unavailable; HeapFree( GetProcessHeap(), 0, buffer ); HOOK_CallHooks( WH_GETMESSAGE, HC_ACTION, flags & PM_REMOVE, (LPARAM)msg, TRUE ); - return TRUE; + return 1; } /* if we get here, we have a sent message; call the window procedure */ @@ -3786,18 +3792,20 @@ static inline void check_for_driver_events( UINT msg ) BOOL WINAPI DECLSPEC_HOTPATCH PeekMessageW( MSG *msg_out, HWND hwnd, UINT first, UINT last, UINT flags ) { MSG msg; + int ret; USER_CheckNotLock(); check_for_driver_events( 0 ); - if (!peek_message( &msg, hwnd, first, last, flags, 0 )) + ret = peek_message( &msg, hwnd, first, last, flags, 0 ); + if (ret < 0) + return FALSE; + else if (!ret) { - DWORD ret; - flush_window_surfaces( TRUE ); ret = wow_handlers.wait_message( 0, NULL, 0, QS_ALLINPUT, 0 ); /* if we received driver events, check again for a pending message */ - if (ret == WAIT_TIMEOUT || !peek_message( &msg, hwnd, first, last, flags, 0 )) return FALSE; + if (ret == WAIT_TIMEOUT || peek_message( &msg, hwnd, first, last, flags, 0 ) <= 0) return FALSE; } check_for_driver_events( msg.message ); @@ -3835,6 +3843,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT { HANDLE server_queue = get_server_queue_handle(); unsigned int mask = QS_POSTMESSAGE | QS_SENDMESSAGE; /* Always selected */ + int ret; USER_CheckNotLock(); check_for_driver_events( 0 ); @@ -3850,10 +3859,12 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT } else mask = QS_ALLINPUT; - while (!peek_message( msg, hwnd, first, last, PM_REMOVE | (mask << 16), mask )) + while (!(ret = peek_message( msg, hwnd, first, last, PM_REMOVE | (mask << 16), mask ))) { wait_objects( 1, &server_queue, INFINITE, mask & (QS_SENDMESSAGE | QS_SMRESULT), mask, 0 ); } + if (ret < 0) + return -1; check_for_driver_events( msg->message ); return (msg->message != WM_QUIT); @@ -3866,7 +3877,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT BOOL WINAPI DECLSPEC_HOTPATCH GetMessageA( MSG *msg, HWND hwnd, UINT first, UINT last ) { if (get_pending_wmchar( msg, first, last, TRUE )) return TRUE; - GetMessageW( msg, hwnd, first, last ); + if (GetMessageW( msg, hwnd, first, last ) < 0) return -1; map_wparam_WtoA( msg, TRUE ); return (msg->message != WM_QUIT); } -- 2.23.0
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- v2: Move the check to the server, partly to avoid making another server call. dlls/user32/tests/msg.c | 42 +++++++++++++++++++++++++++++++++++++++++ server/queue.c | 6 ++++++ 2 files changed, 48 insertions(+) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 058520a37ac..683fc15c208 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -12173,6 +12173,31 @@ todo_wine { qstatus = GetQueueStatus(qs_all_input); ok(qstatus == 0, "wrong qstatus %08x\n", qstatus); } + + PostThreadMessageA(GetCurrentThreadId(), WM_USER, 0, 0); + ret = PeekMessageA(&msg, (HWND)-1, 0, 0, PM_NOREMOVE); + ok(ret == TRUE, "wrong ret %d\n", ret); + ok(msg.message == WM_USER, "wrong message %u\n", msg.message); + ret = GetMessageA(&msg, (HWND)-1, 0, 0); + ok(ret == TRUE, "wrong ret %d\n", ret); + ok(msg.message == WM_USER, "wrong message %u\n", msg.message); + + PostThreadMessageA(GetCurrentThreadId(), WM_USER, 0, 0); + ret = PeekMessageA(&msg, (HWND)1, 0, 0, PM_NOREMOVE); + ok(ret == TRUE, "wrong ret %d\n", ret); + ok(msg.message == WM_USER, "wrong message %u\n", msg.message); + ret = GetMessageA(&msg, (HWND)1, 0, 0); + ok(ret == TRUE, "wrong ret %d\n", ret); + ok(msg.message == WM_USER, "wrong message %u\n", msg.message); + + PostThreadMessageA(GetCurrentThreadId(), WM_USER, 0, 0); + ret = PeekMessageA(&msg, (HWND)0xffff, 0, 0, PM_NOREMOVE); + ok(ret == TRUE, "wrong ret %d\n", ret); + ok(msg.message == WM_USER, "wrong message %u\n", msg.message); + ret = GetMessageA(&msg, (HWND)0xffff, 0, 0); + ok(ret == TRUE, "wrong ret %d\n", ret); + ok(msg.message == WM_USER, "wrong message %u\n", msg.message); + done: trace("signalling to exit\n"); SetEvent(info.hevent[EV_STOP]); @@ -17697,6 +17722,22 @@ static void test_SendMessage_pump(void) DestroyWindow(hwnd); } +static void test_invalid_window(void) +{ + MSG msg; + BOOL ret; + + SetLastError(0xdeadbeef); + ret = GetMessageA(&msg, (HWND)0xdeadbeef, 0, 0); + ok(ret == -1, "wrong ret %d\n", ret); + ok(GetLastError() == ERROR_INVALID_WINDOW_HANDLE, "wrong error %u\n", GetLastError()); + + SetLastError(0xdeadbeef); + ret = PeekMessageA(&msg, (HWND)0xdeadbeef, 0, 0, PM_REMOVE); + ok(!ret, "wrong ret %d\n", ret); + ok(GetLastError() == ERROR_INVALID_WINDOW_HANDLE, "wrong error %u\n", GetLastError()); +} + static void init_funcs(void) { HMODULE hKernel32 = GetModuleHandleA("kernel32.dll"); @@ -17822,6 +17863,7 @@ START_TEST(msg) test_notify_message(); test_SetActiveWindow(); test_restore_messages(); + test_invalid_window(); if (!pTrackMouseEvent) win_skip("TrackMouseEvent is not available\n"); diff --git a/server/queue.c b/server/queue.c index 96587d11d1e..b5e17be18fb 100644 --- a/server/queue.c +++ b/server/queue.c @@ -2394,6 +2394,12 @@ DECL_HANDLER(get_message) reply->active_hooks = get_active_hooks(); + if (get_win && get_win != 1 && get_win != -1 && !get_user_object( get_win, USER_WINDOW )) + { + set_win32_error( ERROR_INVALID_WINDOW_HANDLE ); + return; + } + if (!queue) return; queue->last_get_msg = current_time; if (!filter) filter = QS_ALLINPUT; -- 2.23.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=56524 Your paranoid android. === wvistau64_he (32 bit report) === user32: msg.c:16197: Test failed: 1: WaitForSingleObject failed === w7u (32 bit report) === user32: msg.c:13417: Test failed: 56: ShowWindow(SW_SHOWNOACTIVATE): 5: in msg 0x0047 expecting wParam 0x8170 got 0x1803 msg.c:13417: Test failed: 56: ShowWindow(SW_SHOWNOACTIVATE): 6: the msg 0x0003 was expected, but got msg 0x0009 instead msg.c:13417: Test failed: 56: ShowWindow(SW_SHOWNOACTIVATE): 7: the msg 0x0005 was expected, but got msg 0x0047 instead msg.c:13417: Test failed: 56: ShowWindow(SW_SHOWNOACTIVATE): 13: the msg sequence is not complete: expected 0000 - actual 0003 === w8 (32 bit report) === user32: msg.c:10280: Test failed: did not get expected count for minimum timeout (54 != ~100). === w8adm (32 bit report) === user32: msg.c:13417: Test failed: 46: ShowWindow(SW_RESTORE): 6: in msg 0x0047 expecting wParam 0x8120 got 0x1803 msg.c:13417: Test failed: 46: ShowWindow(SW_RESTORE): 7: the msg 0x0003 was expected, but got msg 0x0009 instead msg.c:13417: Test failed: 46: ShowWindow(SW_RESTORE): 8: the msg 0x0005 was expected, but got msg 0x0047 instead msg.c:13417: Test failed: 46: ShowWindow(SW_RESTORE): 10: the msg sequence is not complete: expected 0000 - actual 0003 msg.c:12862: Test failed: WmMouseHoverSeq: 3: the msg 0x0118 was expected, but got msg 0x001a instead msg.c:12862: Test failed: WmMouseHoverSeq: 4: the msg 0x02a1 was expected, but got msg 0x001a instead msg.c:12862: Test failed: WmMouseHoverSeq: 5: the msg sequence is not complete: expected 0000 - actual 0118 === w1064v1809 (32 bit report) === user32: msg.c:9337: Test failed: Shift+MouseButton press/release: 7: the msg 0x0202 was expected, but got msg 0x0007 instead msg.c:9337: Test failed: Shift+MouseButton press/release: 8: the msg 0x0202 was expected, but got msg 0x0101 instead msg.c:9337: Test failed: Shift+MouseButton press/release: 10: the msg 0x0101 should have been sent msg.c:9337: Test failed: Shift+MouseButton press/release: 11: the msg 0x0101 was expected, but got msg 0x0202 instead msg.c:9337: Test failed: Shift+MouseButton press/release: 12: the msg sequence is not complete: expected 0000 - actual 0202 === debian10 (32 bit report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (32 bit French report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35222 bytes) === debian10 (32 bit Japanese:Japan report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35222 bytes) === debian10 (32 bit Chinese:China report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (32 bit WoW report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (64 bit WoW report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes)
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=56523 Your paranoid android. === debian10 (32 bit report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (32 bit Chinese:China report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (32 bit WoW report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (64 bit WoW report) === user32: msg.c:5148: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes)
participants (2)
-
Marvin -
Zebediah Figura