Re: [PATCH v2 2/7] winebus.sys: Implement removing IOHID devices

[Sebastian Lackner]

On 04.11.2016 13:45, Aric Stewart wrote:
> v2: Suggestions from Ken Thomases
> Signed-off-by: Aric Stewart <aric(a)codeweavers.com>
> ---
>  dlls/winebus.sys/bus_iohid.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> v2-0002-winebus.sys-Implement-removing-IOHID-devices.txt
>
> diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c
> index e47ed13..3cd0b19 100644
> --- a/dlls/winebus.sys/bus_iohid.c
> +++ b/dlls/winebus.sys/bus_iohid.c
> @@ -120,7 +120,9 @@ static DWORD CFNumberToDWORD(CFNumberRef num)
>
> static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev)
>  {
> -    return 0;
> +    ULONG_PTR dev1 = (ULONG_PTR)*(IOHIDDeviceRef*)get_platform_private(device);
> +    ULONG_PTR dev2 = (ULONG_PTR)platform_dev;
> +    return dev1 - dev2;

As already mentioned, the int return value is not sufficient to hold the
difference between the two ULONG_PTR values.
> }
>
> static NTSTATUS get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length)
> @@ -196,6 +198,18 @@ static void handle_DeviceMatchingCallback(void *inContext, IOReturn inResult, vo
>      }
>  }
>
> +static void handle_RemovalCallback(void *inContext, IOReturn inResult, void *inSender, IOHIDDeviceRef inIOHIDDeviceRef)

Similar to patch 1, you could probably use better parameter names here.
> +{
> +    DEVICE_OBJECT *device;
> +    TRACE("OS/X IOHID Device Removed %p\n", inIOHIDDeviceRef);
> +    device = bus_find_hid_device(&iohid_vtbl, inIOHIDDeviceRef);
> +    if (device)
> +    {
> +        IoInvalidateDeviceRelations(device, RemovalRelations);
> +        bus_remove_hid_device(device);
> +    }
> +}
> +
>  /* This puts the relevant run loop for event handling into a WINE thread */
>  static DWORD CALLBACK runloop_thread(VOID *args)
>  {
> @@ -204,6 +218,7 @@ static DWORD CALLBACK runloop_thread(VOID *args)
>
> IOHIDManagerSetDeviceMatching(hid_manager, NULL);
>      IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, handle_DeviceMatchingCallback, driver);
> +    IOHIDManagerRegisterDeviceRemovalCallback(hid_manager, handle_RemovalCallback, driver);

The context parameter is also not really used here.
> IOHIDManagerScheduleWithRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode);
>      if (IOHIDManagerOpen( hid_manager, 0 ) != kIOReturnSuccess)
>      {
> @@ -217,6 +232,7 @@ static DWORD CALLBACK runloop_thread(VOID *args)
>      TRACE("Run Loop exiting\n");
>
> IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, NULL, NULL);
> +    IOHIDManagerRegisterDeviceRemovalCallback(hid_manager, NULL, NULL);
>      IOHIDManagerUnscheduleFromRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode);
>      CFRelease(hid_manager);
>      return 1;