[PATCH] winebus.sys: Implement IOCTL_HID_GET_DEVICE_ATTRIBUTES for hid devices

[Aric Stewart]

On 10/6/16 7:00 AM, Sebastian Lackner wrote:
> On 06.10.2016 04:51, Aric Stewart wrote:
>> Signed-off-by: Aric Stewart <aric at codeweavers.com>
>> ---
>>  dlls/winebus.sys/bus.h      |  1 +
>>  dlls/winebus.sys/bus_udev.c |  1 +
>>  dlls/winebus.sys/main.c     | 41 +++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 43 insertions(+)
>>
>>
>>
>> 0001-winebus.sys-Implement-IOCTL_HID_GET_DEVICE_ATTRIBUTES-.txt
>>
>>
>> diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h
>> index 099558b..342a66b 100644
>> --- a/dlls/winebus.sys/bus.h
>> +++ b/dlls/winebus.sys/bus.h
>> @@ -34,3 +34,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW,
>>                                       const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) DECLSPEC_HIDDEN;
>>  DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN;
>>  void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN;
>> +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
>> diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c
>> index 93e56f3..a01d88e 100644
>> --- a/dlls/winebus.sys/bus_udev.c
>> +++ b/dlls/winebus.sys/bus_udev.c
>> @@ -299,6 +299,7 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry
>>  
>>      udev_driver_obj = driver;
>>      driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch;
>> +    driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch;
> 
> I guess you intentionally did not name it "common_*" because non-HID devices might
> need a different handler, is that correct?
> 

That is correct.

-aric

>>  
>>      if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL)))
>>          goto error;
>> diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
>> index 09a59d6..ef19322 100644
>> --- a/dlls/winebus.sys/main.c
>> +++ b/dlls/winebus.sys/main.c
>> @@ -30,7 +30,9 @@
>>  #include "winternl.h"
>>  #include "winreg.h"
>>  #include "setupapi.h"
>> +#include "winioctl.h"
>>  #include "ddk/wdm.h"
>> +#include "ddk/hidport.h"
>>  #include "wine/debug.h"
>>  #include "wine/unicode.h"
>>  #include "wine/list.h"
>> @@ -346,6 +348,45 @@ NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp)
>>      return status;
>>  }
>>  
>> +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp)
>> +{
>> +    NTSTATUS rc = irp->IoStatus.u.Status;
> 
> I would suggest to use rename the variable to "status".
> 
>> +    IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp);
>> +    struct device_extension *extension = (struct device_extension *)device->DeviceExtension;
>> +
>> +    TRACE("(%p, %p)\n", device, irp);
>> +
>> +    switch(irpsp->Parameters.DeviceIoControl.IoControlCode)
> 
> There is a space missing after "switch".
> 
>> +    {
>> +        case IOCTL_HID_GET_DEVICE_ATTRIBUTES:
>> +        {
>> +            PHID_DEVICE_ATTRIBUTES attr = (PHID_DEVICE_ATTRIBUTES)irp->UserBuffer;
> 
> It would be better to use HID_DEVICE_ATTRIBUTES * instead of the version with the "P" prefix.
> Also, please add a check for Parameters.DeviceIoControl.OutputBufferLength to make sure
> the buffer is big enough.
> 
> 
>> +            TRACE("IOCTL_HID_GET_DEVICE_ATTRIBUTES\n");
>> +
>> +            RtlZeroMemory(attr, sizeof(HID_DEVICE_ATTRIBUTES));
> 
> Is there any specific reason to use RtlZeroMemory instead of memset? Something like
> memset(attr, 0, sizeof(*attr)) would be easier. ;)
> 
>> +            attr->Size = sizeof(HID_DEVICE_ATTRIBUTES);
>> +            attr->VendorID = extension->vid;
>> +            attr->ProductID = extension->pid;
>> +            attr->VersionNumber = extension->version;
>> +            irp->IoStatus.u.Status = rc = STATUS_SUCCESS;
>> +            irp->IoStatus.Information = sizeof(HID_DEVICE_ATTRIBUTES);
>> +            break;
>> +        }
>> +        default:
>> +        {
>> +            ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode;
>> +            FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n",
>> +                  code, code >> 16, (code >> 14) & 3, (code >> 2) & 0xfff, code & 3);
>> +            break;
>> +        }
>> +    }
>> +
>> +    if (rc != STATUS_PENDING)
>> +        IoCompleteRequest( irp, IO_NO_INCREMENT );
> 
> Do you need to handle asynchronous requests in this function? If not you can remove the
> if check. For consistency reasons, you should probably remove the space after "(" and
> before ")" in the IoCompleteRequest call.
> 
>> +
>> +    return rc;
>> +}
>> +
>>  NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path )
>>  {
>>      static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0};
>>
>>
>>
>