On 12.10.2016 12:12, Aric Stewart wrote:
Signed-off-by: Aric Stewart <aric(a)codeweavers.com> --- dlls/winebus.sys/bus.h | 1 + dlls/winebus.sys/bus_udev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++ dlls/winebus.sys/main.c | 11 +++++++ 3 files changed, 82 insertions(+)
0001-winebus.sys-Implement-IOCTL_HID_GET_STRING-for-hidraw.txt
diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h index 9ab242c..9094e17 100644 --- a/dlls/winebus.sys/bus.h +++ b/dlls/winebus.sys/bus.h @@ -24,6 +24,7 @@ typedef struct { int (*compare_platform_device)(DEVICE_OBJECT *device, void *platform_dev); NTSTATUS (*get_reportdescriptor)(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length); + NTSTATUS (*get_string)(DEVICE_OBJECT *device, int index, WCHAR *buffer, DWORD length); } platform_vtbl;
void *get_platform_private(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 22ea07d..4ded568 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -51,7 +51,9 @@ #include "winnls.h" #include "winternl.h" #include "ddk/wdm.h" +#include "ddk/hidtypes.h" #include "wine/debug.h" +#include "wine/unicode.h"
#include "bus.h"
@@ -144,10 +146,78 @@ static NTSTATUS hidraw_get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, #endif }
+static NTSTATUS hidraw_get_string(DEVICE_OBJECT *device, int index, WCHAR *buffer, DWORD length) +{ + struct udev_device *usbdev; + struct platform_private *private = impl_from_DEVICE_OBJECT(device); + + usbdev = udev_device_get_parent_with_subsystem_devtype(private->udev_device, "usb", "usb_device"); + if (usbdev) + { + WCHAR *str = NULL; + switch (index) + { + case HID_STRING_ID_IPRODUCT: + str = get_sysattr_string(usbdev, "product"); + break; + case HID_STRING_ID_IMANUFACTURER: + str = get_sysattr_string(usbdev, "manufacturer"); + break; + case HID_STRING_ID_ISERIALNUMBER: + str = get_sysattr_string(usbdev, "serial"); + break; + default: + ERR("Unhandled string index %i\n", index); + return STATUS_NOT_IMPLEMENTED; + } + if (str) + { + if (strlenW(str) >= length) + { + HeapFree(GetProcessHeap(), 0, str); + return STATUS_BUFFER_TOO_SMALL; + } + strcpyW(buffer, str); + HeapFree(GetProcessHeap(), 0, str); + return STATUS_SUCCESS; + } + else if (length) + buffer[0] = 0; + return STATUS_SUCCESS;
Shouldn't it still return STATUS_BUFFER_TOO_SMALL when length == 0?
+ } + else + { +#ifdef HAVE_LINUX_HIDRAW_H + static char str[MAX_PATH] = {0};
Isn't there a risk of race-conditions that multiple HID devices attempt to use this static buffer at the same time?
+ + switch (index) + { + case HID_STRING_ID_IPRODUCT: + ioctl(private->device_fd, HIDIOCGRAWNAME(MAX_PATH), &str); + break; + case HID_STRING_ID_IMANUFACTURER: + break; + case HID_STRING_ID_ISERIALNUMBER: + break; + default: + ERR("Unhandled string index %i\n", index); + return STATUS_NOT_IMPLEMENTED; + } +#endif + if (str[0])
If HAVE_LINUX_HIDRAW_H is not available this will break compilation (str undeclared).
+ MultiByteToWideChar(CP_ACP, 0, str, -1, buffer, length); + else if (length) + buffer[0] = 0;
It would be better to do length checking also in this case.
+ + return STATUS_SUCCESS; + } +} + static const platform_vtbl hidraw_vtbl = { compare_platform_device, hidraw_get_reportdescriptor, + hidraw_get_string, };
static void try_add_device(struct udev_device *dev) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index ad09ec1..a7b76df 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -421,6 +421,17 @@ NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) irp->IoStatus.Information = length; break; } + case IOCTL_HID_GET_STRING: + { + DWORD index = (DWORD)irpsp->Parameters.DeviceIoControl.Type3InputBuffer;
This will trigger a compilation warning on 64-bit. You have to cast to a ptr-size integer type first, for example: DWORD index = (ULONG_PTR)irpsp->Parameters.DeviceIoControl.Type3InputBuffer; Also, accoding to MSDN, the content of the variable is a composite value: """Parameters.DeviceIoControl.Type3InputBuffer in the I/O stack location of the IRP contains a composite value. The two most significant bytes contain the language ID of the string to be retrieved. The two least significant bytes contain one of the following three constant values..."""
+ TRACE("IOCTL_HID_GET_STRING[%i]\n", index); + irp->IoStatus.u.Status = status = ext->vtbl->get_string(device, + index, (WCHAR*)(irp->UserBuffer), + irpsp->Parameters.DeviceIoControl.OutputBufferLength/sizeof(WCHAR)); + if (status == STATUS_SUCCESS) + irp->IoStatus.Information = (strlenW((WCHAR*)(irp->UserBuffer)) + 1) * sizeof(WCHAR); + break; + } default: { ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode;