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

[Ken Thomases]

On Nov 3, 2016, at 7:14 AM, Aric Stewart  wrote:
> v2: remove Unload code as it will never be triggered
> Signed-off-by: Aric Stewart
>
> 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