Re: [PATCH (resend) 2/8] winebus.sys: Implement adding IOHID devices

[Ken Thomases]

On Nov 3, 2016, at 7:14 AM, Aric Stewart  wrote:
> Signed-off-by: Aric Stewart
>
> dlls/winebus.sys/bus_iohid.c | 102 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c
> index 882b483..f3e282c 100644
> --- a/dlls/winebus.sys/bus_iohid.c
> +++ b/dlls/winebus.sys/bus_iohid.c
> @@ -98,14 +98,114 @@ WINE_DEFAULT_DEBUG_CHANNEL(plugplay);
>  static DRIVER_OBJECT *iohid_driver_obj = NULL;
>  static IOHIDManagerRef hid_manager;
>
> +static const WCHAR busidW[] = {'I','O','H','I','D',0};
> -
>
> +#include "initguid.h"
> +DEFINE_GUID(GUID_DEVCLASS_IOHID, 0x989D309D,0x0470,0x4E1A,0x89,0x38,0x50,0x1F,0x42,0xBD,0x9A,0xCD);
> -
>
> +static void CFStringToWSTR(CFStringRef cstr, LPWSTR wstr, int length)
> +{
> - int len = min(CFStringGetLength(cstr), length-1);
> - CFStringGetCharacters(cstr, CFRangeMake(0, len), wstr);
> - wstr[len] = 0;
>
> +}
> -
>
> +static DWORD CFNumberToDWORD(CFNumberRef num)
> +{
> - DWORD dwNum = 0;
> - if (num)
> -
>    CFNumberGetValue(num, kCFNumberIntType, &dwNum);

The local should be of type "int" to match kCFNumberIntType.
> - return dwNum;
>
> +}
> -

…
> -
>
> +static void handle_DeviceMatchingCallback(void *inContext, IOReturn inResult, void *inSender, IOHIDDeviceRef  inIOHIDDeviceRef)
> +{
> - DEVICE_OBJECT *device;
> - DWORD vid, pid, version;
> - CFStringRef str = NULL;
> - WCHAR serial_string[256];
> - BOOL is_gamepad;
> -
> - TRACE("OS/X IOHID Device Added %p\n", inIOHIDDeviceRef);
> -
> - vid = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDVendorIDKey)));
> - pid = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDProductIDKey)));
> - version = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDVersionNumberKey)));
> - str = IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDSerialNumberKey));
> - if (str) CFStringToWSTR(str, serial_string, 256);

You should use sizeof(serial_string) / sizeof(WCHAR) instead of 256 here.
> -
> - is_gamepad = (IOHIDDeviceConformsTo(inIOHIDDeviceRef, kHIDPage_GenericDesktop, kHIDUsage_GD_GamePad) ||
> -
>   IOHIDDeviceConformsTo(inIOHIDDeviceRef, kHIDPage_GenericDesktop, kHIDUsage_GD_Joystick));
> -
> - device = bus_create_hid_device(iohid_driver_obj, busidW, vid, pid, version, 0, str?serial_string:NULL, is_gamepad, &GUID_DEVCLASS_IOHID, &iohid_vtbl, sizeof(inIOHIDDeviceRef));
> - if (!device)
> -
>    ERR("Failed to create device\n");
> - else
> - {
> -
>    IOHIDDeviceRef* ext = get_platform_private(device);
> -
>    *ext = inIOHIDDeviceRef;
> -
>    IoInvalidateDeviceRelations(device, BusRelations);
> - }
>
> +}
> -
>
> /* This puts the relevent run loop for event handleing into a WINE thread */
>  static DWORD CALLBACK runloop_thread(VOID *args)
>  {
>      CFRunLoopRef run_loop;
> - DRIVER_OBJECT *driver = (DRIVER_OBJECT*)args;
> run_loop = CFRunLoopGetCurrent();

Minor style nit: the variable declarations would normally be grouped and separated from statements.  Of course, the declaration and setting of run_loop could be merged into one line (in the previous patch) to eliminate the distinction.
>  IOHIDManagerSetDeviceMatching(hid_manager, NULL);
> - IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, handle_DeviceMatchingCallback, driver);
> IOHIDManagerScheduleWithRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode);
> if (IOHIDManagerOpen( hid_manager, 0 ) != kIOReturnSuccess)
> {
>
> @@ -130,7 +230,7 @@ NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registr
>      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)))
> - if (!(run_loop_handle = CreateThread(NULL, 0, runloop_thread, driver, 0, NULL)))
> {
>     ERR("Failed to initialize IOHID Manager thread\n");
>     iohid_driver_obj = NULL;