Skip to content

Correctly get all HID descriptors #116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hughsie wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Correctly get all HID descriptors #116

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions gusb/gusb-device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,7 @@ _g_usb_device_load(GUsbDevice *self, JsonObject *json_object, GError **error)
for (guint i = 0; i < json_array_get_length(json_array); i++) {
JsonNode *node_tmp = json_array_get_element(json_array, i);
JsonObject *obj_tmp = json_node_get_object(node_tmp);
g_autoptr(GUsbInterface) iface =
g_object_new(G_USB_TYPE_INTERFACE, NULL);
g_autoptr(GUsbInterface) iface = g_object_new(G_USB_TYPE_INTERFACE, NULL);
if (!_g_usb_interface_load(iface, obj_tmp, error))
return FALSE;
g_ptr_array_add(priv->interfaces, g_object_ref(iface));
Expand Down Expand Up @@ -1345,6 +1344,7 @@ g_usb_device_get_hid_descriptor_for_interface(GUsbDevice *self, GUsbInterface *i
gsize buf2sz;
guint16 buf2szle = 0;
g_autofree guint8 *buf2 = NULL;
g_autoptr(GError) error_local = NULL;

extra = g_usb_interface_get_extra(intf);
if (extra == NULL) {
Expand Down Expand Up @@ -1384,9 +1384,10 @@ g_usb_device_get_hid_descriptor_for_interface(GUsbDevice *self, GUsbInterface *i
g_usb_interface_get_number(intf));
return NULL;
}
g_debug("get 0x%x bytes of HID descriptor on iface 0x%x",
g_debug("get 0x%x bytes of HID descriptor on iface=0x%x, index=0x%x",
(guint)buf2sz,
g_usb_interface_get_number(intf));
g_usb_interface_get_number(intf),
g_usb_interface_get_index(intf));

/* get HID descriptor */
buf2 = g_malloc0(buf2sz);
Expand All @@ -1395,15 +1396,24 @@ g_usb_device_get_hid_descriptor_for_interface(GUsbDevice *self, GUsbInterface *i
G_USB_DEVICE_REQUEST_TYPE_STANDARD,
G_USB_DEVICE_RECIPIENT_INTERFACE,
LIBUSB_REQUEST_GET_DESCRIPTOR,
LIBUSB_DT_REPORT << 8,
g_usb_interface_get_number(intf),
(LIBUSB_DT_REPORT << 8) |
g_usb_interface_get_number(intf),
g_usb_interface_get_index(intf),
buf2,
buf2sz,
&actual_length,
5000,
NULL,
error)) {
g_prefix_error(error, "failed to get HID report descriptor: ");
&error_local)) {
if (g_error_matches(error_local,
G_USB_DEVICE_ERROR,
G_USB_DEVICE_ERROR_NOT_SUPPORTED)) {
g_debug("ignoring: %s", error_local->message);
Copy link
Collaborator

@nacho nacho Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of feel that it would be more correct to report an error than an empty buffer

Copy link
Owner Author

@hughsie hughsie Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially thought so too; but I've yet to find a device that reports two HID descriptors and actually allows you to request the non-default one. I suspect we have a deeper bug somewhere, but interestingly sudo lsusb -vvvv also shows the same fault.

return g_bytes_new(NULL, 0);
}
g_propagate_prefixed_error(error,
g_steal_pointer(&error_local),
"failed to get HID report descriptor: ");
return NULL;
}
if (actual_length < buf2sz) {
Expand Down Expand Up @@ -1469,6 +1479,8 @@ g_usb_device_get_hid_descriptors(GUsbDevice *self, GError **error)
blob = g_usb_device_get_hid_descriptor_for_interface(self, intf, error);
if (blob == NULL)
return NULL;
if (g_bytes_get_size(blob) == 0)
continue;
g_ptr_array_add(priv->hid_descriptors, g_steal_pointer(&blob));
}
priv->hid_descriptors_valid = TRUE;
Expand Down