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  wrote:
> Signed-off-by: Aric Stewart
>
> 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;
>
> }