[3/3] winebus.sys: Add implementation for device removal. (v9)

Aric Stewart aric at codeweavers.com
Mon Oct 3 10:33:22 CDT 2016


Signed-off-by: Aric Stewart <aric at codeweavers.com>

On 10/3/16 6:29 AM, Sebastian Lackner wrote:
> From: Aric Stewart <aric at codeweavers.com>
> 
> Signed-off-by: Aric Stewart <aric at codeweavers.com>
> Signed-off-by: Sebastian Lackner <sebastian at fds-team.de>
> ---
> 
> Second part of 126774. Besides splitting, I've also done the following changes:
> 
> * Rename find_hid_device -> bus_find_hid_device, for consistency with other bus_*_hid_*() functions.
> * Do not append timestamp to the device names. To make the code more robust, I've modified
>   bus_remove_hid_device() to defer the release of the pnp_device pointer. If there are more
>   problems I would prefer to fix them properly, adding the timestamp seems more like a hack to me.
> * Some style improvements.
> 
> @Aric: I'm not sure if you are aware of it, but there is currently still a bug somewhere which
> causes a crash during RemovalRelations when trying to remove a driver which was never added properly.
> I feel like the right place to fix that is somewhere in ntoskrnl. Are you aware of this issue?
> 
>  dlls/winebus.sys/bus.h      |   16 +++++++--
>  dlls/winebus.sys/bus_udev.c |   38 ++++++++++++++++++++--
>  dlls/winebus.sys/main.c     |   74 ++++++++++++++++++++++++++++++++++++++------
>  3 files changed, 111 insertions(+), 17 deletions(-)
> 
> diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h
> index dcb50f2..099558b 100644
> --- a/dlls/winebus.sys/bus.h
> +++ b/dlls/winebus.sys/bus.h
> @@ -19,8 +19,18 @@
>  /* Busses */
>  NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) DECLSPEC_HIDDEN;
>  
> +/* Native device function table */
> +typedef struct
> +{
> +    int (*compare_platform_device)(DEVICE_OBJECT *device, void *platform_dev);
> +} platform_vtbl;
> +
> +void *get_platform_private(DEVICE_OBJECT *device) 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;
> +DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, WORD vid, WORD pid,
> +                                     DWORD version, DWORD uid, const WCHAR *serialW, BOOL is_gamepad,
> +                                     const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) DECLSPEC_HIDDEN;
> +DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) 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 b1d6c3f..d798c53 100644
> --- a/dlls/winebus.sys/bus_udev.c
> +++ b/dlls/winebus.sys/bus_udev.c
> @@ -61,6 +61,16 @@ static const WCHAR hidraw_busidW[] = {'H','I','D','R','A','W',0};
>  #include "initguid.h"
>  DEFINE_GUID(GUID_DEVCLASS_HIDRAW, 0x3def44ad,0x242e,0x46e5,0x82,0x6d,0x70,0x72,0x13,0xf3,0xaa,0x81);
>  
> +struct platform_private
> +{
> +    struct udev_device *udev_device;
> +};
> +
> +static inline struct platform_private *impl_from_DEVICE_OBJECT(DEVICE_OBJECT *device)
> +{
> +    return (struct platform_private *)get_platform_private(device);
> +}
> +
>  static DWORD get_sysattr_dword(struct udev_device *dev, const char *sysattr, int base)
>  {
>      const char *attr = udev_device_get_sysattr_value(dev, sysattr);
> @@ -88,6 +98,18 @@ static WCHAR *get_sysattr_string(struct udev_device *dev, const char *sysattr)
>      return dst;
>  }
>  
> +static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev)
> +{
> +    struct udev_device *dev1 = impl_from_DEVICE_OBJECT(device)->udev_device;
> +    struct udev_device *dev2 = platform_dev;
> +    return strcmp(udev_device_get_syspath(dev1), udev_device_get_syspath(dev2));
> +}
> +
> +static const platform_vtbl hidraw_vtbl =
> +{
> +    compare_platform_device,
> +};
> +
>  static void try_add_device(struct udev_device *dev)
>  {
>      DWORD vid = 0, pid = 0, version = 0;
> @@ -123,12 +145,15 @@ static void try_add_device(struct udev_device *dev)
>      subsystem = udev_device_get_subsystem(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);
> +        device = bus_create_hid_device(udev_driver_obj, hidraw_busidW, vid, pid, version, 0, serial, FALSE,
> +                                       &GUID_DEVCLASS_HIDRAW, &hidraw_vtbl, sizeof(struct platform_private));
>      }
>  
>      if (device)
> -        udev_device_ref(dev);
> +    {
> +        impl_from_DEVICE_OBJECT(device)->udev_device = udev_device_ref(dev);
> +        IoInvalidateDeviceRelations(device, BusRelations);
> +    }
>      else
>          WARN("Ignoring device %s with subsystem %s\n", debugstr_a(devnode), subsystem);
>  
> @@ -137,7 +162,12 @@ static void try_add_device(struct udev_device *dev)
>  
>  static void try_remove_device(struct udev_device *dev)
>  {
> -    /* FIXME */
> +    DEVICE_OBJECT *device = bus_find_hid_device(&hidraw_vtbl, dev);
> +    if (!device) return;
> +
> +    dev = impl_from_DEVICE_OBJECT(device)->udev_device;
> +    bus_remove_hid_device(device);
> +    udev_device_unref(dev);
>  }
>  
>  static void build_initial_deviceset(void)
> diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
> index 1364eab..09a59d6 100644
> --- a/dlls/winebus.sys/main.c
> +++ b/dlls/winebus.sys/main.c
> @@ -47,13 +47,16 @@ struct pnp_device
>  
>  struct device_extension
>  {
> -    void *native;  /* Must be the first member of the structure */
> +    struct pnp_device *pnp_device;
>  
>      WORD vid, pid;
>      DWORD uid, version, index;
>      BOOL is_gamepad;
>      WCHAR *serial;
>      const WCHAR *busid;  /* Expected to be a static constant */
> +
> +    const platform_vtbl *vtbl;
> +    BYTE platform_private[1];
>  };
>  
>  static CRITICAL_SECTION device_list_cs;
> @@ -80,6 +83,12 @@ static inline WCHAR *strdupW(const WCHAR *src)
>      return dst;
>  }
>  
> +void *get_platform_private(DEVICE_OBJECT *device)
> +{
> +    struct device_extension *ext = (struct device_extension *)device->DeviceExtension;
> +    return ext->platform_private;
> +}
> +
>  static DWORD get_vidpid_index(WORD vid, WORD pid)
>  {
>      struct pnp_device *ptr;
> @@ -157,9 +166,9 @@ static WCHAR *get_compatible_ids(DEVICE_OBJECT *device)
>      return dst;
>  }
>  
> -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)
> +DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, WORD vid, WORD pid,
> +                                     DWORD version, DWORD uid, const WCHAR *serialW, BOOL is_gamepad,
> +                                     const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size)
>  {
>      static const WCHAR device_name_fmtW[] = {'\\','D','e','v','i','c','e','\\','%','s','#','%','p',0};
>      struct device_extension *ext;
> @@ -169,16 +178,18 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW,
>      WCHAR dev_name[256];
>      HDEVINFO devinfo;
>      NTSTATUS status;
> +    DWORD length;
>  
> -    TRACE("(%p, %s, %p, %04x, %04x, %u, %u, %s, %u, %s)\n", driver, debugstr_w(busidW), native,
> -          vid, pid, version, uid, debugstr_w(serialW), is_gamepad, debugstr_guid(class));
> +    TRACE("(%p, %s, %04x, %04x, %u, %u, %s, %u, %s, %p, %u)\n", driver, debugstr_w(busidW), vid, pid,
> +          version, uid, debugstr_w(serialW), is_gamepad, debugstr_guid(class), vtbl, platform_data_size);
>  
>      if (!(pnp_dev = HeapAlloc(GetProcessHeap(), 0, sizeof(*pnp_dev))))
>          return NULL;
>  
> -    sprintfW(dev_name, device_name_fmtW, busidW, native);
> +    sprintfW(dev_name, device_name_fmtW, busidW, pnp_dev);
>      RtlInitUnicodeString(&nameW, dev_name);
> -    status = IoCreateDevice(driver, sizeof(*ext), &nameW, 0, 0, FALSE, &device);
> +    length = FIELD_OFFSET(struct device_extension, platform_private[platform_data_size]);
> +    status = IoCreateDevice(driver, length, &nameW, 0, 0, FALSE, &device);
>      if (status)
>      {
>          FIXME("failed to create device error %x\n", status);
> @@ -190,7 +201,7 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW,
>  
>      /* fill out device_extension struct */
>      ext = (struct device_extension *)device->DeviceExtension;
> -    ext->native     = native;
> +    ext->pnp_device = pnp_dev;
>      ext->vid        = vid;
>      ext->pid        = pid;
>      ext->uid        = uid;
> @@ -199,6 +210,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;
> @@ -226,10 +238,52 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW,
>      else
>          ERR("failed to get ClassDevs: %x\n", GetLastError());
>  
> -    IoInvalidateDeviceRelations(device, BusRelations);
>      return device;
>  }
>  
> +DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev)
> +{
> +    struct pnp_device *dev;
> +    DEVICE_OBJECT *ret = NULL;
> +
> +    TRACE("(%p, %p)\n", vtbl, platform_dev);
> +
> +    EnterCriticalSection(&device_list_cs);
> +    LIST_FOR_EACH_ENTRY(dev, &pnp_devset, struct pnp_device, entry)
> +    {
> +        struct device_extension *ext = (struct device_extension *)dev->device->DeviceExtension;
> +        if (ext->vtbl != vtbl) continue;
> +        if (ext->vtbl->compare_platform_device(dev->device, platform_dev) == 0)
> +        {
> +            ret = dev->device;
> +            break;
> +        }
> +    }
> +    LeaveCriticalSection(&device_list_cs);
> +
> +    TRACE("returning %p\n", ret);
> +    return ret;
> +}
> +
> +void bus_remove_hid_device(DEVICE_OBJECT *device)
> +{
> +    struct device_extension *ext = (struct device_extension *)device->DeviceExtension;
> +    struct pnp_device *pnp_device = ext->pnp_device;
> +
> +    TRACE("(%p)\n", device);
> +
> +    EnterCriticalSection(&device_list_cs);
> +    list_remove(&pnp_device->entry);
> +    LeaveCriticalSection(&device_list_cs);
> +
> +    IoInvalidateDeviceRelations(device, RemovalRelations);
> +    HeapFree(GetProcessHeap(), 0, ext->serial);
> +    IoDeleteDevice(device);
> +
> +    /* pnp_device must be released after the device is gone */
> +    HeapFree(GetProcessHeap(), 0, pnp_device);
> +}
> +
>  static NTSTATUS handle_IRP_MN_QUERY_ID(DEVICE_OBJECT *device, IRP *irp)
>  {
>      NTSTATUS status = irp->IoStatus.u.Status;
> 



More information about the wine-patches mailing list