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;
>
>
>