windows: fix winusb_get_device_string failing for strict USB devices#1796
windows: fix winusb_get_device_string failing for strict USB devices#1796sonatique wants to merge 1 commit intolibusb:masterfrom
Conversation
|
I thought libusb_get_device_string() would not create I/O and use cached values from the OS, but here I see a DeviceIoControl() call. Does this call create I/O? Or is libusb_get_device_string() not I/O free on Windows anyway? |
As per the comments in PR #1532, no better ways for Windows now. From PR #1532. |
|
Not able to recreate the issues for Issue #1794, but at least no regressions now. |
|
Changing my Windows 11 system to Singapore Chinese -- still the same. |
|
BTW, another interesting finding. For thebuilt-in USB devices of the Acer Swift Go 14 laptop (2024 model) like the WebCAM, USBview and USBTreeView will not be able to get the string descriptors since they are in Low Power mode. But all the different versions of libusb testlibusb example will be able to get the string descriptors. Looks like libusb Windows will wake up the device and get the string descriptors. I have tested libusb git, this PR, 1.0.29 release and 1.0.28 release. |
|
I will check the behavior of the dual boot Ubuntu 26.04 beta version as well tomorrow. |
| sd.req.ConnectionIndex = (ULONG)dev->port_number; | ||
| sd.req.SetupPacket.bmRequest = LIBUSB_ENDPOINT_IN; | ||
| sd.req.SetupPacket.bRequest = LIBUSB_REQUEST_GET_DESCRIPTOR; | ||
| sd.req.SetupPacket.wValue = (LIBUSB_DT_STRING << 8) | 0; |
There was a problem hiding this comment.
I believe this is just a notation to clarify that whatever goes in the LSB is 0, to follow a pattern of a << 8 | b.
There was a problem hiding this comment.
Indeed: Writing (LIBUSB_DT_STRING << 8) | 0 makes the structure explicit: we're requesting string descriptor index 0 (the language table).
But it is a matter of preference, we may remove it if this is more confusing than self-explaining.
or maybe I can add: /* index 0 = language table */
There was a problem hiding this comment.
Adding that index 0 = language table comment would be nice.
Thanks for the pointer! At least it should work without opening the device (which on Windows means opening an interface). |
|
@smarvonohr @mcuee @seanm @tormodvolden @Fishi96 : I think I am doing too many things at the same time: I totally forgot we had #1790 pending (and even that I had commented it) when I saw #1794 which was open afterward, and since @mcuee requested my help, I jumped into it and didn't make the connection immediately. I thus try to solve #1794 on my own, resulting in this PR. And now, when browsing recent PRs because something looked familiar after all, I realized some of the issues were already fixed by #1790, so I could have built upon it rather than creating a whole new PR, this may look a bit rude, sorry. Anyway, now that we are in this situation, I compared both PR, and found that, in my opinion this one is actually going a little bit further in solving the problems of #1790:
So for these reason I would recommend merging this one and unfortunately dropping #1790, but I am open to any other solution. |
5b0c1b8 to
186ea85
Compare
Three bugs in winusb_get_device_string caused libusb_get_device_string to return empty strings instead of actual device strings on Windows: 1. The function used wIndex=0 (invalid LANGID) when requesting string descriptors via IOCTL_USB_GET_DESCRIPTOR_FROM_NODE_CONNECTION. While some devices tolerate LANGID 0, strict USB devices correctly STALL the request (Windows error 31). Fix: first fetch string descriptor 0 to get the device's language table (USB 2.0 section 9.6.7), then use the primary LANGID for actual string requests. The LANGID is cached per device to avoid redundant IOCTLs. 2. On IOCTL failure, the function returned 0 instead of a negative LIBUSB_ERROR code. The caller in libusb_get_device_string only checks for rv < 0, so 0 was treated as success: an empty string was permanently cached, and subsequent calls returned 1 (empty string + null terminator) without ever retrying. Fix: return LIBUSB_ERROR_IO on both error paths. 3. When a device has no string descriptor of a given type (descriptor index is 0 in the device descriptor), the function returned 0 instead of an error. This caused an empty string to be cached and returned as success, while Linux and Darwin return an error in the same situation. Fix: return LIBUSB_ERROR_NOT_FOUND for consistency with other backends (using Darwing value which seemed more precise than Linux. Closes libusb#1794
|
Note: amended my commit to fix this: #1794 (comment) |
I agree. |
|
Looks good to me. |
[EDITED for description of the third issue]
Three bugs in winusb_get_device_string caused libusb_get_device_string
to return empty strings instead of actual device strings on Windows:
The function used wIndex=0 (invalid LANGID) when requesting string
descriptors via IOCTL_USB_GET_DESCRIPTOR_FROM_NODE_CONNECTION. While
some devices tolerate LANGID 0, strict USB devices correctly STALL
the request (Windows error 31). Fix: first fetch string descriptor 0
to get the device's language table (USB 2.0 section 9.6.7), then use
the primary LANGID for actual string requests. The LANGID is cached
per device to avoid redundant IOCTLs.
On IOCTL failure, the function returned 0 instead of a negative
LIBUSB_ERROR code. The caller in libusb_get_device_string only
checks for rv < 0, so 0 was treated as success: an empty string
was permanently cached, and subsequent calls returned 1 (empty
string + null terminator) without ever retrying. Fix: return
LIBUSB_ERROR_IO on both error paths.
When a device has no string descriptor of a given type
(descriptor index is 0 in the device descriptor), the function
returned 0 instead of an error. This caused an empty string to be
cached and returned as success, while Linux and Darwin return an
error in the same situation. Fix: return LIBUSB_ERROR_NOT_FOUND
for consistency with other backends (using Darwing value which seemed
more precise than Linux.
Closes #1794