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
>
> 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;