Re: [PATCH v2 1/8] winebus.sys: Add OS/X IOHID bus

[Ken Thomases]

On Nov 3, 2016, at 7:14 AM, Aric Stewart <aric(a)codeweavers.com> wrote:
> v2: remove Unload code as it will never be triggered
> Signed-off-by: Aric Stewart <aric(a)codeweavers.com>
> ---
> dlls/winebus.sys/Makefile.in |   3 +-
> dlls/winebus.sys/bus.h       |   1 +
> dlls/winebus.sys/bus_iohid.c | 151 +++++++++++++++++++++++++++++++++++++++++++
> dlls/winebus.sys/main.c      |   3 +
> 4 files changed, 157 insertions(+), 1 deletion(-)
> create mode 100644 dlls/winebus.sys/bus_iohid.c
>
> diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c
> new file mode 100644
> index 0000000..882b483
> --- /dev/null
> +++ b/dlls/winebus.sys/bus_iohid.c
> @@ -0,0 +1,151 @@

…
> +
> +WINE_DEFAULT_DEBUG_CHANNEL(plugplay);
> +#ifdef HAVE_IOHIDMANAGERCREATE
> +
> +static DRIVER_OBJECT *iohid_driver_obj = NULL;
> +static IOHIDManagerRef hid_manager;
> +
> +/* This puts the relevent run loop for event handleing into a WINE thread */

Spelling: "relevant", "handling".
> +static DWORD CALLBACK runloop_thread(VOID *args)
> +{
> +    CFRunLoopRef run_loop;
> +
> +    run_loop = CFRunLoopGetCurrent();
> +
> +    IOHIDManagerSetDeviceMatching(hid_manager, NULL);
> +    IOHIDManagerScheduleWithRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode);
> +    if (IOHIDManagerOpen( hid_manager, 0 ) != kIOReturnSuccess)
> +    {
> +        ERR("Couldn't open IOHIDManager.\n");
> +        IOHIDManagerUnscheduleFromRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode);
> +        CFRelease(hid_manager);
> +        return 0;
> +    }
> +
> +    CFRunLoopRun();
> +    TRACE("Run Loop exiting\n");

Under what circumstances might CFRunLoopRun() return (if any)?  You should presumably unschedule the hid_manager from the run loop and release it in this path, too.
> +    return 1;
> +}
> +
> +NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path)
> +{
> +    HANDLE run_loop_handle;
> +
> +    TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer));
> +
> +    iohid_driver_obj = driver;
> +    driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch;
> +    driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch;
> +    hid_manager = IOHIDManagerCreate(kCFAllocatorDefault, 0L);
> +    if (!(run_loop_handle = CreateThread(NULL, 0, runloop_thread, NULL, 0, NULL)))
> +    {
> +        ERR("Failed to initialize IOHID Manager thread\n");
> +        iohid_driver_obj = NULL;

You should CFRelease(hid_manager) in this failure path.
> +        return STATUS_UNSUCCESSFUL;
> +    }
> +

You should CloseHandle(run_loop_handle) before returning.
> +    return STATUS_SUCCESS;
> +}
> +
> +#else
> +
> +NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path)
> +{
> +    WARN("IOHID Support not compiled into Wine.\n");
> +    return STATUS_NOT_IMPLEMENTED;
> +}
> +
> +#endif /* HAVE_IOHIDMANAGERCREATE */
> diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
> index 791ccbb..ed95435 100644
> --- a/dlls/winebus.sys/main.c
> +++ b/dlls/winebus.sys/main.c
> @@ -628,10 +628,13 @@ NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path )
>  {
>      static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0};
>      static UNICODE_STRING udev = {sizeof(udevW) - sizeof(WCHAR), sizeof(udevW), (WCHAR *)udevW};
> +    static const WCHAR iohidW[] = {'\\','D','r','i','v','e','r','\\','I','O','H','I','D',0};
> +    static UNICODE_STRING iohid = {sizeof(iohidW) - sizeof(WCHAR), sizeof(iohidW), (WCHAR *)iohidW};
>
> TRACE( "(%p, %s)\n", driver, debugstr_w(path->Buffer) );
>
> IoCreateDriver(&udev, udev_driver_init);
> +    IoCreateDriver(&iohid, iohid_driver_init);
>
> return STATUS_SUCCESS;
>  }

In general, this patch is a bit weird in that it doesn't do anything.  It could maybe be combined with the next one.  *shrug*

-Ken