[PATCH v3] winebus.sys: Watch for hid raw device addition and removal
Aric Stewart
aric at codeweavers.com
Wed Sep 21 07:07:45 CDT 2016
On 9/21/16 5:59 AM, Sebastian Lackner wrote:
> On 19.09.2016 15:35, Aric Stewart wrote:
>> v2: Correct poll timeout
>> Style changes
>> v3: vtable style instead of a callback
>>
>>
>> Signed-off-by: Aric Stewart <aric at codeweavers.com>
>> ---
>> dlls/winebus.sys/bus.h | 11 +++++-
>> dlls/winebus.sys/bus_udev.c | 92 ++++++++++++++++++++++++++++++++++++++++++++-
>> dlls/winebus.sys/main.c | 52 ++++++++++++++++++++++++-
>> 3 files changed, 152 insertions(+), 3 deletions(-)
>>
>
> Would you mind sharing some of the follow up patches, so that I can check if the vtbl solution
> really looks better than the previous approach?
>
Sure, Here is the full patchset for hidraw.
https://www.codeweavers.com/xfer/aric/HID/Hid_Patches_20160921.tgz
access-code: sLcm7EZ7
I did not include the iohid and linux event patches in that given set as I know they still need cleaning up and I am working on that. Also many of the later patches probably still need some cleaning up.
-aric
>>
>>
>> v3-0004-winebus.sys-Watch-for-hid-raw-device-addition-and-r.txt
>>
>>
>> diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h
>> index dcb50f2..cf10e80 100644
>> --- a/dlls/winebus.sys/bus.h
>> +++ b/dlls/winebus.sys/bus.h
>> @@ -19,8 +19,17 @@
>> /* Busses */
>> NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) DECLSPEC_HIDDEN;
>>
>> +/* Native device function table */
>> +typedef struct {
>> + int(*compare_native_device)(DEVICE_OBJECT *device, DEVICE_OBJECT *other);
>> +} platform_vtbl;
>> +
>> +void *get_native_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN;
>> +DEVICE_OBJECT *find_hid_device(const WCHAR* busidW, void *native) DECLSPEC_HIDDEN;
>> +
>> /* HID Plug and Play Bus */
>> NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
>> DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, void *native, WORD vid,
>> WORD pid, DWORD version, DWORD uid, const WCHAR *serialW, BOOL is_gamepad,
>> - const GUID *class) DECLSPEC_HIDDEN;
>> + const GUID *class, platform_vtbl* vtbl) DECLSPEC_HIDDEN;
>> +void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN;
>> diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c
>> index 1525861..35b198a 100644
>> --- a/dlls/winebus.sys/bus_udev.c
>> +++ b/dlls/winebus.sys/bus_udev.c
>> @@ -26,6 +26,11 @@
>> #ifdef HAVE_UNISTD_H
>> # include <unistd.h>
>> #endif
>> +
>> +#ifdef HAVE_SYS_POLL_H
>> +# include <sys/poll.h>
>> +#endif
>> +
>> #ifdef HAVE_LIBUDEV_H
>> # include <libudev.h>
>> #endif
>> @@ -82,6 +87,17 @@ static WCHAR *get_sysattr_string(struct udev_device *dev, const char *sysattr)
>> return dst;
>> }
>>
>> +static int compare_native_device(DEVICE_OBJECT *device, DEVICE_OBJECT *other)
>> +{
>> + struct udev_device *dev1 = get_native_device(device);
>> + struct udev_device *dev2 = get_native_device(other);
>> + return (strcmp(udev_device_get_syspath(dev1), udev_device_get_syspath(dev2)));
>> +}
>> +
>> +static platform_vtbl hidraw_vtbl = {
>
> This should be made const.
>
>> + compare_native_device,
>> +};
>> +
>> static void try_add_device(struct udev_device *dev)
>> {
>> DWORD vid = 0, pid = 0, version = 0;
>> @@ -118,7 +134,7 @@ static void try_add_device(struct udev_device *dev)
>> if (strcmp(subsystem, "hidraw") == 0)
>> {
>> device = bus_create_hid_device(udev_driver_obj, hidraw_busidW, dev, vid, pid,
>> - version, 0, serial, FALSE, &GUID_DEVCLASS_HIDRAW);
>> + version, 0, serial, FALSE, &GUID_DEVCLASS_HIDRAW, &hidraw_vtbl);
>> }
>>
>> if (device)
>> @@ -164,6 +180,77 @@ static void build_initial_deviceset(void)
>> udev_enumerate_unref(enumerate);
>> }
>>
>> +/* This is our main even loop for reading devices */
>> +static DWORD CALLBACK deviceloop_thread(void *args)
>> +{
>> + struct pollfd plfds[1];
>> + struct udev_monitor *monitor = NULL;
>> +
>> + monitor = udev_monitor_new_from_netlink(udev_context, "udev");
>> + if (!monitor)
>> + {
>> + WARN("Unable to get udev monitor, device insertion and removal cannot be tracked\n");
>> + goto exit;
>> + }
>> + if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw", NULL) < 0)
>> + {
>> + WARN("Failed to add 'hidraw' subsystem to monitor\n");
>> + goto exit;
>> + }
>> +
>> + if (udev_monitor_enable_receiving(monitor) < 0)
>> + {
>> + WARN("Failed to start monitoring\n");
>> + goto exit;
>> + }
>> +
>> + plfds[0].fd = udev_monitor_get_fd(monitor);
>> + plfds[0].events = POLLIN;
>> + if (plfds[0].fd < 0)
>> + {
>> + WARN("Failed to get monitor fd\n");
>> + goto exit;
>> + }
>> + /* Run the event loop */
>> + while (1)
>> + {
>> + struct udev_device *dev;
>> + const char* action;
>> +
>> + if (poll(plfds, 1, -1) <= 0) continue;
>> +
>> + TRACE("udev reports device changes\n");
>> + dev = udev_monitor_receive_device(monitor);
>> + if (!dev)
>> + {
>> + FIXME("Failed to get device that has changed\n");
>> + continue;
>> + }
>> + action = udev_device_get_action(dev);
>> + if (!action)
>> + WARN("No action recieved\n");
>> + else if (strcmp(action, "add") == 0)
>> + try_add_device(dev);
>> + else if (strcmp(action, "remove") == 0)
>> + {
>> + DEVICE_OBJECT *device = find_hid_device(hidraw_busidW, dev);
>> + if (device)
>> + {
>> + struct udev_device *original = get_native_device(device);
>> + bus_remove_hid_device(device);
>> + udev_device_unref(original);
>> + }
>> + }
>> + udev_device_unref(dev);
>> + }
>> +
>> +exit:
>> + TRACE("Monitor thread exiting\n");
>> + if (monitor)
>> + udev_monitor_unref(monitor);
>> + return 1;
>> +}
>> +
>> NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path)
>> {
>> TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer));
>> @@ -178,6 +265,9 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry
>> driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch;
>>
>> build_initial_deviceset();
>> +
>> + CreateThread(NULL, 0, deviceloop_thread, NULL, 0, NULL);
>
> It would be better to check the result here (at least to print an ERR or something like that).
>
>> +
>> return STATUS_SUCCESS;
>> }
>>
>> diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
>> index 1364eab..c7418e3 100644
>> --- a/dlls/winebus.sys/main.c
>> +++ b/dlls/winebus.sys/main.c
>> @@ -54,6 +54,7 @@ struct device_extension
>> BOOL is_gamepad;
>> WCHAR *serial;
>> const WCHAR *busid; /* Expected to be a static constant */
>> + platform_vtbl *vtbl;
>> };
>>
>> static CRITICAL_SECTION device_list_cs;
>> @@ -159,7 +160,7 @@ static WCHAR *get_compatible_ids(DEVICE_OBJECT *device)
>>
>> DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, void *native, WORD vid,
>> WORD pid, DWORD version, DWORD uid, const WCHAR *serialW, BOOL is_gamepad,
>> - const GUID *class)
>> + const GUID *class, platform_vtbl *vtbl)
>> {
>> static const WCHAR device_name_fmtW[] = {'\\','D','e','v','i','c','e','\\','%','s','#','%','p',0};
>> struct device_extension *ext;
>> @@ -199,6 +200,7 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW,
>> ext->is_gamepad = is_gamepad;
>> ext->serial = strdupW(serialW);
>> ext->busid = busidW;
>> + ext->vtbl = vtbl;
>>
>> /* add to list of pnp devices */
>> pnp_dev->device = device;
>> @@ -265,6 +267,54 @@ static NTSTATUS handle_IRP_MN_QUERY_ID(DEVICE_OBJECT *device, IRP *irp)
>> return status;
>> }
>>
>> +void *get_native_device(DEVICE_OBJECT *device)
>> +{
>> + struct device_extension *ext = (struct device_extension*)device->DeviceExtension;
>> + return ext->native;
>> +}
>> +
>> +DEVICE_OBJECT *find_hid_device(const WCHAR* busidW, void *native)
>> +{
>> + struct pnp_device *dev, *ptr;
>> + DEVICE_OBJECT *ret = NULL;
>> +
>> + EnterCriticalSection(&device_list_cs);
>> + LIST_FOR_EACH_ENTRY_SAFE(dev, ptr, &pnp_devset, struct pnp_device, entry)
>> + {
>> + struct device_extension *ext = (struct device_extension*)dev->device->DeviceExtension;
>> + if ((strcmpW(busidW, ext->busid) == 0) &&
>> + (ext->vtbl->compare_native_device(dev->device, native) == 0))
>> + {
>> + TRACE("Found as device %p\n", dev->device);
>> + ret = dev->device;
>> + break;
>> + }
>> + }
>> + LeaveCriticalSection(&device_list_cs);
>> + return ret;
>> +}
>> +
>> +void bus_remove_hid_device(DEVICE_OBJECT *device)
>> +{
>> + struct pnp_device *dev, *ptr;
>> + TRACE("Remove hid device %p\n", device);
>> + EnterCriticalSection(&device_list_cs);
>> + LIST_FOR_EACH_ENTRY_SAFE(dev, ptr, &pnp_devset, struct pnp_device, entry)
>> + {
>> + struct device_extension *ext = (struct device_extension*)dev->device->DeviceExtension;
>> + if (device == dev->device)
>> + {
>> + list_remove(&dev->entry);
>> + IoInvalidateDeviceRelations(dev->device, RemovalRelations);
>> + HeapFree(GetProcessHeap(), 0, ext->serial);
>> + IoDeleteDevice(dev->device);
>> + HeapFree(GetProcessHeap(), 0, dev);
>> + break;
>> + }
>> + }
>> + LeaveCriticalSection(&device_list_cs);
>> +}
>> +
>> NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp)
>> {
>> NTSTATUS status = irp->IoStatus.u.Status;
>>
>>
>>
>
More information about the wine-devel
mailing list