New subject: [PATCH v3] winebus.sys: Watch for hid raw device addition and removal

21 Sep 2016

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@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?
...
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;

    

Re: [PATCH v3] winebus.sys: Watch for hid raw device addition and removal

Signed-off-by: Aric Stewart aric@codeweavers.com

Signed-off-by: Aric Stewart aric@codeweavers.com