windows: change MAX_STRING_WCHARS to 126 (#627)

Win32 HID API doc says: For USB devices, the maximum string length is 126 wide characters (not including the terminating NULL character).

For certain USB devices, using a buffer larger or equal to 127 wchars results in successful completion of HID API functions, but a broken string is stored in the output buffer. This behaviour persists even if HID API is bypassed and HID IOCTLs are passed to the HID driver directly (IOCTL_HID_GET_MANUFACTURER_STRING, IOCTL_HID_GET_PRODUCT_STRING, etc).

So, the buffer MUST NOT exceed 126 wchars.

windows: refactor ULONGLONG hid_internal_get_info(...) -> hid_internal_detect_bus_type_result hid_internal_detect_bus_type(...)

hid_internal_detect_bus_type is now only responsible for detection of the bus type; rename it accordingly. Also, mixing an internal flag and DEV_INST into an ULONGLONG retval feels kinda hackish; use a cleaner approach instead (add an internal flag to help distinguishing between BLUETOOTH and BLE devices, then clear it once we are done).

Author: vovkos

windows/hid.c

@@ -69,6 +69,15 @@ typedef LONG NTSTATUS;
 /* BLUETOOTH_DEVICE_NAME_SIZE from bluetoothapis.h is 256 */
 #define MAX_STRING_WCHARS 256
 
+/* For certain USB devices, using a buffer larger or equal to 127 wchars results
+   in successful completion of HID API functions, but a broken string is stored
+   in the output buffer. This behaviour persists even if HID API is bypassed and
+   HID IOCTLs are passed to the HID driver directly. Therefore, for USB devices,
+   the buffer MUST NOT exceed 126 WCHARs.
+*/
+
+#define MAX_STRING_WCHARS_USB 126
+
 static struct hid_api_version api_version = {
 	.major = HID_API_VERSION_MAJOR,
 	.minor = HID_API_VERSION_MINOR,
@@ -590,11 +599,22 @@ static void hid_internal_get_ble_info(struct hid_device_info* dev, DEVINST dev_n
 	}
 }
 
-static void hid_internal_get_info(const wchar_t* interface_path, struct hid_device_info* dev)
+/* Unfortunately, HID_API_BUS_xxx constants alone aren't enough to distinguish between BLUETOOTH and BLE */
+
+#define HID_API_BUS_FLAG_BLE 0x01
+
+typedef struct hid_internal_detect_bus_type_result_ {
+	DEVINST dev_node;
+	hid_bus_type bus_type;
+	unsigned int bus_flags;
+} hid_internal_detect_bus_type_result;
+
+static hid_internal_detect_bus_type_result hid_internal_detect_bus_type(const wchar_t* interface_path)
 {
 	wchar_t *device_id = NULL, *compatible_ids = NULL;
 	CONFIGRET cr;
 	DEVINST dev_node;
+	hid_internal_detect_bus_type_result result = { 0 };
 
 	/* Get the device id from interface path */
 	device_id = hid_internal_get_device_interface_property(interface_path, &DEVPKEY_Device_InstanceId, DEVPROP_TYPE_STRING);
@@ -625,42 +645,45 @@ static void hid_internal_get_info(const wchar_t* interface_path, struct hid_devi
 		   https://docs.microsoft.com/windows-hardware/drivers/hid/plug-and-play-support
 		   https://docs.microsoft.com/windows-hardware/drivers/install/standard-usb-identifiers */
 		if (wcsstr(compatible_id, L"USB") != NULL) {
-			dev->bus_type = HID_API_BUS_USB;
-			hid_internal_get_usb_info(dev, dev_node);
+			result.bus_type = HID_API_BUS_USB;
 			break;
 		}
 
 		/* Bluetooth devices
 		   https://docs.microsoft.com/windows-hardware/drivers/bluetooth/installing-a-bluetooth-device */
 		if (wcsstr(compatible_id, L"BTHENUM") != NULL) {
-			dev->bus_type = HID_API_BUS_BLUETOOTH;
+			result.bus_type = HID_API_BUS_BLUETOOTH;
 			break;
 		}
 
 		/* Bluetooth LE devices */
 		if (wcsstr(compatible_id, L"BTHLEDEVICE") != NULL) {
-			dev->bus_type = HID_API_BUS_BLUETOOTH;
-			hid_internal_get_ble_info(dev, dev_node);
+			result.bus_type = HID_API_BUS_BLUETOOTH;
+			result.bus_flags |= HID_API_BUS_FLAG_BLE;
 			break;
 		}
 
 		/* I2C devices
 		   https://docs.microsoft.com/windows-hardware/drivers/hid/plug-and-play-support-and-power-management */
 		if (wcsstr(compatible_id, L"PNP0C50") != NULL) {
-			dev->bus_type = HID_API_BUS_I2C;
+			result.bus_type = HID_API_BUS_I2C;
 			break;
 		}
 
 		/* SPI devices
 		   https://docs.microsoft.com/windows-hardware/drivers/hid/plug-and-play-for-spi */
 		if (wcsstr(compatible_id, L"PNP0C51") != NULL) {
-			dev->bus_type = HID_API_BUS_SPI;
+			result.bus_type = HID_API_BUS_SPI;
 			break;
 		}
 	}
+
+	result.dev_node = dev_node;
+
 end:
 	free(device_id);
 	free(compatible_ids);
+	return result;
 }
 
 static char *hid_internal_UTF16toUTF8(const wchar_t *src)
@@ -699,7 +722,10 @@ static struct hid_device_info *hid_internal_get_device_info(const wchar_t *path,
 	HIDD_ATTRIBUTES attrib;
 	PHIDP_PREPARSED_DATA pp_data = NULL;
 	HIDP_CAPS caps;
-	wchar_t string[MAX_STRING_WCHARS];
+	wchar_t string[MAX_STRING_WCHARS + 1];
+	ULONG len;
+	ULONG size;
+	hid_internal_detect_bus_type_result detect_bus_type_result;
 
 	/* Create the record. */
 	dev = (struct hid_device_info*)calloc(1, sizeof(struct hid_device_info));
@@ -733,25 +759,46 @@ static struct hid_device_info *hid_internal_get_device_info(const wchar_t *path,
 		HidD_FreePreparsedData(pp_data);
 	}
 
+	/* detect bus type before reading string descriptors */
+	detect_bus_type_result = hid_internal_detect_bus_type(path);
+	dev->bus_type = detect_bus_type_result.bus_type;
+
+	len = dev->bus_type == HID_API_BUS_USB ? MAX_STRING_WCHARS_USB : MAX_STRING_WCHARS;
+	string[len] = L'\0';
+	size = len * sizeof(wchar_t);
+
 	/* Serial Number */
 	string[0] = L'\0';
-	HidD_GetSerialNumberString(handle, string, sizeof(string));
-	string[MAX_STRING_WCHARS - 1] = L'\0';
+	HidD_GetSerialNumberString(handle, string, size);
 	dev->serial_number = _wcsdup(string);
 
 	/* Manufacturer String */
 	string[0] = L'\0';
-	HidD_GetManufacturerString(handle, string, sizeof(string));
-	string[MAX_STRING_WCHARS - 1] = L'\0';
+	HidD_GetManufacturerString(handle, string, size);
 	dev->manufacturer_string = _wcsdup(string);
 
 	/* Product String */
 	string[0] = L'\0';
-	HidD_GetProductString(handle, string, sizeof(string));
-	string[MAX_STRING_WCHARS - 1] = L'\0';
+	HidD_GetProductString(handle, string, size);
 	dev->product_string = _wcsdup(string);
 
-	hid_internal_get_info(path, dev);
+	/* now, the portion that depends on string descriptors */
+	switch (dev->bus_type) {
+	case HID_API_BUS_USB:
+		hid_internal_get_usb_info(dev, detect_bus_type_result.dev_node);
+		break;
+
+	case HID_API_BUS_BLUETOOTH:
+		if (detect_bus_type_result.bus_flags & HID_API_BUS_FLAG_BLE)
+			hid_internal_get_ble_info(dev, detect_bus_type_result.dev_node);
+		break;
+
+	case HID_API_BUS_UNKNOWN:
+	case HID_API_BUS_SPI:
+	case HID_API_BUS_I2C:
+		/* shut down -Wswitch */
+		break;
+	}
 
 	return dev;
 }
@@ -1356,7 +1403,12 @@ int HID_API_EXPORT_CALL HID_API_CALL hid_get_indexed_string(hid_device *dev, int
 {
 	BOOL res;
 
-	res = HidD_GetIndexedString(dev->device_handle, string_index, string, sizeof(wchar_t) * (DWORD) MIN(maxlen, MAX_STRING_WCHARS));
+	if (dev->device_info && dev->device_info->bus_type == HID_API_BUS_USB && maxlen > MAX_STRING_WCHARS_USB) {
+		string[MAX_STRING_WCHARS_USB] = L'\0';
+		maxlen = MAX_STRING_WCHARS_USB;
+	}
+
+	res = HidD_GetIndexedString(dev->device_handle, string_index, string, (ULONG)maxlen * sizeof(wchar_t));
 	if (!res) {
 		register_winapi_error(dev, L"HidD_GetIndexedString");
 		return -1;