[PATCH v4 1/5] server: Added support for tracking registered device notifications
Signed-off-by: Micah N Gorrell <mgorrell(a)codeweavers.com> --- server/protocol.def | 23 +++++++++ server/user.h | 3 +- server/window.c | 112 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) diff --git a/server/protocol.def b/server/protocol.def index e450388c17..1645d09d5f 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3692,6 +3692,29 @@ struct handle_info @END +/* Register a handle for device notifications */ +(a)REQ(register_device_notification) + user_handle_t recipient; /* handle to a window or service that will receive device events */ +(a)REPLY + user_handle_t handle; /* handle associated with the registration */ +(a)END + + +/* Unregister device notifications for a handle */ +(a)REQ(unregister_device_notification) + user_handle_t notification; /* handle that was previously returned by register_device_notification */ +(a)END + + +/* Return a list of handles that have registered for device notifications */ +(a)REQ(get_device_notifications) +(a)REPLY + int count; /* total count of registered handles */ + VARARG(handles,user_handles); /* registered handles */ +(a)END + + + /* Make the current process a system process */ @REQ(make_process_system) @REPLY diff --git a/server/user.h b/server/user.h index eb1b7ce1e4..282cb63339 100644 --- a/server/user.h +++ b/server/user.h @@ -36,7 +36,8 @@ enum user_object { USER_WINDOW = 1, USER_HOOK, - USER_CLIENT /* arbitrary client handle */ + USER_CLIENT, /* arbitrary client handle */ + USER_DEVNOTIFY }; #define DESKTOP_ATOM ((atom_t)32769) diff --git a/server/window.c b/server/window.c index c9b131cba5..80ac897ea6 100644 --- a/server/window.c +++ b/server/window.c @@ -129,6 +129,19 @@ static struct window *taskman_window; #define WINPTR_TOPMOST ((struct window *)3L) #define WINPTR_NOTOPMOST ((struct window *)4L) +struct device_notification +{ + user_handle_t recipient; /* the window handle to send notifications to */ + user_handle_t handle; /* full handle for this window */ + struct process *process; /* process that owns this notification handle */ + struct list entry; /* entry in global notifications list */ + + /* FIXME: Add support for notification filters */ +}; + +/* global list of handles that have registered for device notifications */ +static struct list device_notifications = LIST_INIT(device_notifications); + /* retrieve a pointer to a window from its handle */ static inline struct window *get_window( user_handle_t handle ) { @@ -2880,3 +2893,102 @@ DECL_HANDLER(set_window_layered_info) } else set_win32_error( ERROR_INVALID_WINDOW_HANDLE ); } + +/* retrieve a pointer to a device_notification from its handle */ +static inline struct device_notification *get_device_notification( user_handle_t handle ) +{ + struct device_notification *ret = get_user_object( handle, USER_DEVNOTIFY ); + if (!ret) set_win32_error( ERROR_INVALID_HANDLE ); + return ret; +} + +/* create a new device notification structure and link it into the global list */ +static struct device_notification *register_device_notification( struct window *recipient ) +{ + struct device_notification *notification; + + if (!recipient) + { + return NULL; + } + + if (!(notification = mem_alloc( sizeof(*notification) ))) goto failed; + if (!(notification->handle = alloc_user_handle( notification, USER_DEVNOTIFY ))) goto failed; + + notification->recipient = recipient->handle; + notification->process = current->process; + /* FIXME: Add support for notification filters */ + + list_add_tail( &device_notifications, ¬ification->entry ); + + return notification; +failed: + if (notification) + { + if (notification->handle) free_user_handle( notification->handle ); + free( notification ); + } + + return NULL; +} + +static void unregister_device_notification( struct device_notification *notification) +{ + list_remove( ¬ification->entry ); + free_user_handle( notification->handle ); + memset( notification, 0x55, sizeof(*notification) ); + free( notification ); +} + +static int get_device_notifications( struct user_handle_array *array ) +{ + struct device_notification *notification; + + LIST_FOR_EACH_ENTRY( notification, &device_notifications, struct device_notification, entry ) + { + /* FIXME: Implement notification filtering */ + if (!add_handle_to_array( array, notification->recipient )) return 0; + } + + return 1; +} + +/* register for device notifications */ +DECL_HANDLER(register_device_notification) +{ + struct device_notification *notification = NULL; + struct window *recipient = get_window( req->recipient ); + + reply->handle = 0; + if (!recipient) return; + + if (!(notification = register_device_notification( recipient ))) return; + + reply->handle = notification->handle; +} + +DECL_HANDLER(unregister_device_notification) +{ + struct device_notification *notification = get_device_notification( req->notification ); + if (notification) { + if (notification->process == current->process) unregister_device_notification(notification); + else set_error( STATUS_ACCESS_DENIED ); + } +} + +DECL_HANDLER(get_device_notifications) +{ + struct user_handle_array array; + data_size_t len; + + array.handles = NULL; + array.count = 0; + array.total = 0; + + if (!get_device_notifications( &array )) return; + + reply->count = array.count; + len = min( get_reply_max_size(), array.count * sizeof(user_handle_t) ); + if (len) set_reply_data_ptr( array.handles, len ); + else free( array.handles ); +} -- 2.21.0
Signed-off-by: Micah N Gorrell <mgorrell(a)codeweavers.com> --- include/winuser.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/winuser.h b/include/winuser.h index 60bc473225..165f835b7d 100644 --- a/include/winuser.h +++ b/include/winuser.h @@ -3088,7 +3088,9 @@ typedef struct tagTRACKMOUSEEVENT { typedef PVOID HDEVNOTIFY; typedef HDEVNOTIFY *PHDEVNOTIFY; -#define DEVICE_NOTIFY_WINDOW_HANDLE 0x00000000 +#define DEVICE_NOTIFY_WINDOW_HANDLE 0x00000000 +#define DEVICE_NOTIFY_SERVICE_HANDLE 0x00000001 +#define DEVICE_NOTIFY_ALL_INTERFACE_CLASSES 0x00000004 /* used for GetWindowInfo() */ -- 2.21.0
Signed-off-by: Micah N Gorrell <mgorrell(a)codeweavers.com> --- dlls/user32/misc.c | 74 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/dlls/user32/misc.c b/dlls/user32/misc.c index 1a03d70dde..648f597837 100644 --- a/dlls/user32/misc.c +++ b/dlls/user32/misc.c @@ -33,6 +33,7 @@ #include "winternl.h" #include "controls.h" #include "user_private.h" +#include "wine/server.h" #include "wine/unicode.h" #include "wine/debug.h" @@ -363,11 +364,9 @@ DWORD WINAPI RegisterTasklist (DWORD x) * * See RegisterDeviceNotificationW. */ -HDEVNOTIFY WINAPI RegisterDeviceNotificationA(HANDLE hnd, LPVOID notifyfilter, DWORD flags) +HDEVNOTIFY WINAPI RegisterDeviceNotificationA(HANDLE hRecipient, LPVOID pNotificationFilter, DWORD dwFlags) { - FIXME("(hwnd=%p, filter=%p,flags=0x%08x) returns a fake device notification handle!\n", - hnd,notifyfilter,flags ); - return (HDEVNOTIFY) 0xcafecafe; + return RegisterDeviceNotificationW( hRecipient, pNotificationFilter, dwFlags ); } /*********************************************************************** @@ -395,9 +394,52 @@ HDEVNOTIFY WINAPI RegisterDeviceNotificationA(HANDLE hnd, LPVOID notifyfilter, D */ HDEVNOTIFY WINAPI RegisterDeviceNotificationW(HANDLE hRecipient, LPVOID pNotificationFilter, DWORD dwFlags) { - FIXME("(hwnd=%p, filter=%p,flags=0x%08x) returns a fake device notification handle!\n", + HWND ret = 0; + + TRACE("(hwnd=%p, filter=%p,flags=0x%08x)\n", hRecipient,pNotificationFilter,dwFlags ); - return (HDEVNOTIFY) 0xcafeaffe; + + if (dwFlags & DEVICE_NOTIFY_ALL_INTERFACE_CLASSES) + { + dwFlags &= ~DEVICE_NOTIFY_ALL_INTERFACE_CLASSES; + pNotificationFilter = NULL; + } + + /* Wine broadcasts WM_DEVICECHANGE anyway, so registering without a handle + * is a not needed and can be ignored. + */ + if (!hRecipient) + return (HDEVNOTIFY) 0xcafeaffe; + + switch (dwFlags) { + case DEVICE_NOTIFY_WINDOW_HANDLE: + break; + + case DEVICE_NOTIFY_SERVICE_HANDLE: + FIXME("Support for service handles is not yet implemented! Returns a fake device notification handle!\n"); + return (HDEVNOTIFY) 0xcafeaffe; + + default: + SetLastError(ERROR_INVALID_FLAGS); + return 0; + } + + /* This implementation is not overly concerned with sending too many + * messages, so support for filters is not yet implemented. + */ + if (pNotificationFilter) + FIXME("Notification filters are not yet implemented! All WM_DEVICECHANGE messages will be sent.\n"); + + SERVER_START_REQ( register_device_notification ) + { + req->recipient = wine_server_user_handle( hRecipient ); + + wine_server_call( req ); + ret = wine_server_ptr_handle( reply->handle ); + } + SERVER_END_REQ; + + return (HDEVNOTIFY) ret; } /*********************************************************************** @@ -406,8 +448,24 @@ HDEVNOTIFY WINAPI RegisterDeviceNotificationW(HANDLE hRecipient, LPVOID pNotific */ BOOL WINAPI UnregisterDeviceNotification(HDEVNOTIFY hnd) { - FIXME("(handle=%p), STUB!\n", hnd); - return TRUE; + unsigned int res = 0; + + TRACE("(hnd=%p)\n", hnd); + + /* A fake device notification handle is returned in some cases */ + if ((HDEVNOTIFY) 0xcafeaffe == hnd) + return TRUE; + + SERVER_START_REQ( unregister_device_notification ) + { + req->notification = wine_server_user_handle( hnd ); + + res = wine_server_call( req ); + } + SERVER_END_REQ; + + if (res) SetLastError( RtlNtStatusToDosError( res ) ); + return !res; } /*********************************************************************** -- 2.21.0
Signed-off-by: Micah N Gorrell <mgorrell(a)codeweavers.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index b5d89f6ee5..dad89016ab 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -1712,6 +1712,45 @@ static NTSTATUS create_device_symlink( DEVICE_OBJECT *device, UNICODE_STRING *sy return ret; } +/******************************************************************* + * list_device_notifications + * + * Build an array of windows which have been registered to receive device + * notifications. The array must be freed with HeapFree. Returns NULL when no + * registered windows are found. + */ +static HWND *list_device_notifications( void ) +{ + HWND *list; + int i, count, size = 128; + + for (;;) + { + count = 0; + + if (!(list = heap_alloc( size * sizeof(HWND) ))) break; + + SERVER_START_REQ( get_device_notifications ) + { + wine_server_set_reply( req, list, (size-1) * sizeof(user_handle_t) ); + if (!wine_server_call( req )) count = reply->count; + } + SERVER_END_REQ; + if (count && count < size) + { + /* start from the end since HWND is potentially larger than user_handle_t */ + for (i = count - 1; i >= 0; i--) + list[i] = wine_server_ptr_handle( ((user_handle_t *)list)[i] ); + list[count] = 0; + return list; + } + heap_free( list ); + if (!count) break; + size = count + 1; /* restart with a large enough buffer */ + } + return NULL; +} + /*********************************************************************** * IoSetDeviceInterfaceState (NTOSKRNL.EXE.@) */ @@ -1739,6 +1778,8 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable NTSTATUS ret; GUID class; ULONG len; + HWND *registrations; + unsigned int i; TRACE("(%s, %d)\n", debugstr_us(name), enable); @@ -1822,6 +1863,19 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable BroadcastSystemMessageW( BSF_FORCEIFHUNG | BSF_QUERY, NULL, WM_DEVICECHANGE, enable ? DBT_DEVICEARRIVAL : DBT_DEVICEREMOVECOMPLETE, (LPARAM)broadcast ); + if ((registrations = list_device_notifications( ))) + { + for (i = 0; registrations[i]; i++) + { + TRACE("Sending WM_DEVICECHANGE to registered window %p\n", registrations[i]); + SendMessageTimeoutW( registrations[i], WM_DEVICECHANGE, + enable ? DBT_DEVICEARRIVAL : DBT_DEVICEREMOVECOMPLETE, + (LPARAM)broadcast, SMTO_ABORTIFHUNG, 2000, NULL ); + } + + heap_free( registrations ); + } + heap_free( broadcast ); } return ret; -- 2.21.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=53098 Your paranoid android. === debian9 (64 bit WoW report) === user32: msg.c:8713: Test failed: WaitForSingleObject failed 102 msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000
Signed-off-by: Micah N Gorrell <mgorrell(a)codeweavers.com> --- dlls/user32/tests/Makefile.in | 1 + dlls/user32/tests/misc.c | 163 ++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 dlls/user32/tests/misc.c diff --git a/dlls/user32/tests/Makefile.in b/dlls/user32/tests/Makefile.in index 7149dc824e..da55fa58e5 100644 --- a/dlls/user32/tests/Makefile.in +++ b/dlls/user32/tests/Makefile.in @@ -15,6 +15,7 @@ C_SRCS = \ input.c \ listbox.c \ menu.c \ + misc.c \ monitor.c \ msg.c \ resource.c \ diff --git a/dlls/user32/tests/misc.c b/dlls/user32/tests/misc.c new file mode 100644 index 0000000000..16fed49f8d --- /dev/null +++ b/dlls/user32/tests/misc.c @@ -0,0 +1,163 @@ +/* + * Unit tests for misc functions + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <assert.h> +#include <stdlib.h> +#include <stdarg.h> +#include <stdio.h> + +#include "wine/test.h" +#include "windef.h" +#include "winbase.h" +#include "winuser.h" +#include "dbt.h" +#include "initguid.h" +#include "ddk/hidclass.h" + +static const WCHAR mainwindowclassW[] = {'M','a','i','n','W','i','n','d','o','w','C','l','a','s','s',0}; + +static HWND create_message_window(void) +{ + static const WCHAR message_windowW[] = {'m','e','s','s','a','g','e',' ','w','i','n','d','o','w',0}; + HWND hwnd; + + hwnd = CreateWindowExW(0, mainwindowclassW, message_windowW, 0, + 0, 0, 0, 0, HWND_MESSAGE, 0, 0, NULL); + ok( hwnd != 0, "CreateWindowExW with parent HWND_MESSAGE failed\n" ); + return hwnd; +} + +static void prepare_dbh(DEV_BROADCAST_DEVICEINTERFACE_W *dbh) +{ + if (!dbh) return; + + memset(dbh, 0, sizeof(*dbh)); + + dbh->dbcc_size = sizeof(*dbh); + dbh->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; + dbh->dbcc_classguid = GUID_DEVINTERFACE_HID; +} + +static void test_register_device_notification(void) +{ + HDEVNOTIFY hnotify1, hnotify2; + DEV_BROADCAST_DEVICEINTERFACE_W dbh; + HWND hwnd = create_message_window(); + + prepare_dbh(&dbh); + + /* Test RegisterDeviceNotification behavior */ + + /* Prior to Windows 8 a NULL recipient handle caused a failure, but more + * recent versions of windows allow it. + */ + hnotify1 = RegisterDeviceNotificationW( NULL, &dbh, DEVICE_NOTIFY_WINDOW_HANDLE ); + /* ok( hnotify1 != 0, "RegisterDeviceNotificationW failed when called with a NULL recipient window handle\n" ); */ + if ( hnotify1 != 0 ) + ok( UnregisterDeviceNotification( hnotify1 ), "UnregisterDeviceNotification failed with a valid handle\n" ); + + hnotify1 = RegisterDeviceNotificationW( hwnd, &dbh, DEVICE_NOTIFY_WINDOW_HANDLE ); + ok( hnotify1 != 0, "RegisterDeviceNotificationW failed when called with a message only window as recipient\n" ); + + hnotify2 = RegisterDeviceNotificationW( hwnd, &dbh, DEVICE_NOTIFY_WINDOW_HANDLE ); + ok( hnotify2 != 0, "RegisterDeviceNotificationW failed when called with a window that has already been registered as a recipient\n" ); + + ok( UnregisterDeviceNotification( hnotify1 ), "UnregisterDeviceNotification failed with a valid handle\n" ); + ok( UnregisterDeviceNotification( hnotify2 ), "UnregisterDeviceNotification failed with a valid handle\n" ); + + hnotify1 = RegisterDeviceNotificationW( hwnd, &dbh, 0xffff ); + ok( hnotify1 == 0, "RegisterDeviceNotificationW accepted invalid flags\n" ); + + DestroyWindow(hwnd); +} + +static void test_unregister_device_notification(void) +{ + HDEVNOTIFY hnotify; + DEV_BROADCAST_DEVICEINTERFACE_W dbh; + HWND hwnd = create_message_window(); + + prepare_dbh(&dbh); + + hnotify = RegisterDeviceNotificationW( hwnd, &dbh, DEVICE_NOTIFY_WINDOW_HANDLE ); + ok( hnotify != 0, "RegisterDeviceNotificationW failed when called with a message only window as recipient\n" ); + + /* Destroy the window before calling UnregisterDeviceNotification */ + DestroyWindow(hwnd); + + ok( UnregisterDeviceNotification( hnotify ), "UnregisterDeviceNotification failed with a valid handle\n" ); + ok( !UnregisterDeviceNotification( hnotify ), "UnregisterDeviceNotification succeeded with an already released handle\n" ); + ok( !UnregisterDeviceNotification( NULL ), "UnregisterDeviceNotification succeeded with NULL handle\n" ); +} + +static void test_device_notification_handles( char *argv0 ) +{ + HDEVNOTIFY hnotify; + DEV_BROADCAST_DEVICEINTERFACE_W dbh; + HWND hwnd = create_message_window(); + PROCESS_INFORMATION info; + STARTUPINFOA startup; + char cmd[MAX_PATH]; + + prepare_dbh(&dbh); + + hnotify = RegisterDeviceNotificationW( hwnd, &dbh, DEVICE_NOTIFY_WINDOW_HANDLE ); + ok( hnotify != 0, "RegisterDeviceNotificationW failed when called with a message only window as recipient\n" ); + + /* Ensure that another process can't mess with our handle */ + sprintf(cmd, "%s misc unregister_device_notification %p", argv0, hnotify); + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + ok(CreateProcessA(NULL, cmd, NULL, NULL, FALSE, 0, NULL, NULL, + &startup, &info), "CreateProcess failed.\n"); + winetest_wait_child_process(info.hProcess); + CloseHandle(info.hProcess); + CloseHandle(info.hThread); + + ok( UnregisterDeviceNotification( hnotify ), "UnregisterDeviceNotification failed with a valid handle\n" ); +} + +START_TEST(misc) +{ + char **argv; + int argc = winetest_get_mainargs( &argv ); + WNDCLASSEXW cls; + + if (argc==4 && !strcmp(argv[2], "unregister_device_notification")) + { + HDEVNOTIFY hnotify; + + sscanf(argv[3], "%p", &hnotify); + ok( !UnregisterDeviceNotification( hnotify ), "UnregisterDeviceNotification succeeded on a handle owned by another process\n" ); + return; + } + + memset(&cls, 0, sizeof(cls)); + cls.cbSize = sizeof(cls); + cls.hInstance = 0; + cls.lpszClassName = mainwindowclassW; + cls.lpfnWndProc = DefWindowProcW; + + if (!RegisterClassExW(&cls)) assert(0); + + test_register_device_notification(); + test_unregister_device_notification(); + test_device_notification_handles(argv[0]); + + /* FIXME: Find a way to trigger a device notification for testing */ +} -- 2.21.0
Micah N Gorrell <mgorrell(a)codeweavers.com> writes:
diff --git a/server/user.h b/server/user.h index eb1b7ce1e4..282cb63339 100644 --- a/server/user.h +++ b/server/user.h @@ -36,7 +36,8 @@ enum user_object { USER_WINDOW = 1, USER_HOOK, - USER_CLIENT /* arbitrary client handle */ + USER_CLIENT, /* arbitrary client handle */ + USER_DEVNOTIFY
I don't think these should be user handles, especially since they don't work across processes. But if they are, you'd need some mechanism to clean them up when the client dies. To be honest I'm not convinced these should be in the server at all. Probably they could just as well be registered directly with the service that sends the notification. -- Alexandre Julliard julliard(a)winehq.org
Thank you. I'll make another attempt. On Wed, Jun 12, 2019 at 05:13:32PM +0200, Alexandre Julliard wrote:
Micah N Gorrell <mgorrell(a)codeweavers.com> writes:
diff --git a/server/user.h b/server/user.h index eb1b7ce1e4..282cb63339 100644 --- a/server/user.h +++ b/server/user.h @@ -36,7 +36,8 @@ enum user_object { USER_WINDOW = 1, USER_HOOK, - USER_CLIENT /* arbitrary client handle */ + USER_CLIENT, /* arbitrary client handle */ + USER_DEVNOTIFY
I don't think these should be user handles, especially since they don't work across processes. But if they are, you'd need some mechanism to clean them up when the client dies.
To be honest I'm not convinced these should be in the server at all. Probably they could just as well be registered directly with the service that sends the notification.
-- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Marvin -
Micah N Gorrell