Re: [PATCH (resend) 5/8] winebus.sys: IOCTL_HID_GET_STRING for iohid

[Ken Thomases]

On Nov 3, 2016, at 7:15 AM, Aric Stewart <aric(a)codeweavers.com> wrote:
> Signed-off-by: Aric Stewart <aric(a)codeweavers.com>
> ---
> dlls/winebus.sys/bus_iohid.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c
> index 26243fb..6e3c044 100644
> --- a/dlls/winebus.sys/bus_iohid.c
> +++ b/dlls/winebus.sys/bus_iohid.c
> @@ -88,6 +88,7 @@
>  #include "winternl.h"
>  #include "winioctl.h"
>  #include "ddk/wdm.h"
> +#include "ddk/hidtypes.h"
>  #include "wine/debug.h"
>
> #include "bus.h"
> @@ -143,7 +144,28 @@ static NTSTATUS get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD
>
> static NTSTATUS get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length)
>  {
> -    return STATUS_NOT_IMPLEMENTED;
> +    IOHIDDeviceRef dev = *(IOHIDDeviceRef*)get_platform_private(device);
> +    CFStringRef str = NULL;

It's not necessary to initialize str here.
> +    switch (index)
> +    {
> +        case HID_STRING_ID_IPRODUCT:
> +            str = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDProductKey));
> +            break;
> +        case HID_STRING_ID_IMANUFACTURER:
> +            str = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDManufacturerKey));
> +            break;
> +        case HID_STRING_ID_ISERIALNUMBER:
> +            str = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDSerialNumberKey));
> +            break;
> +        default:
> +            ERR("Unknown string index\n");
> +            return STATUS_NOT_IMPLEMENTED;
> +    }
> +
> +    if (str)
> +        CFStringToWSTR(str, buffer, length);

What if str is NULL?  Should the function return a different result code, or should it set the caller's buffer to the empty string, or…? Certainly, it shouldn't just return success but leave the caller's buffer with junk.
> +
> +    return STATUS_SUCCESS;
>  }