Hi Uri, On Wed, Nov 12, 2014 at 6:01 PM, Uri Lublin <uril@xxxxxxxxxx> wrote: > Hi Fabiano, > > Please see some comments below. > > On 11/11/2014 04:29 PM, Fabiano Fidêncio wrote: >> >> As we only can filter USB devices by their Classes and sometimes it is >> not enough (eg: I do not want to have Keyboard and Mouse, but want to >> have have Joysticks, being all of them part of HID Class), provide to >> the applications a way match the device itself with the desired Class, >> SubClass and Protocol, so the applications refine the filter provided >> by usbredir. > > > Note that the filter can act upon VID/PID such that any specific device can > have its own rule, e.g. -1, VID, PID, -1, 1 Yeah, but still hard to filter for all keyboard devices, for instance. > >> --- >> This patch was written based on >> https://bugzilla.gnome.org/show_bug.cgi?id=698430 >> Any suggestion to have a shorter short-log is welcome :-) >> --- >> gtk/map-file | 1 + >> gtk/spice-glib-sym-file | 1 + >> gtk/usb-device-manager.c | 96 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> gtk/usb-device-manager.h | 1 + >> 4 files changed, 99 insertions(+) >> >> diff --git a/gtk/map-file b/gtk/map-file >> index 9f8d04e..6bbf407 100644 >> --- a/gtk/map-file >> +++ b/gtk/map-file >> @@ -124,6 +124,7 @@ spice_usb_device_manager_get_devices; >> spice_usb_device_manager_get_devices_with_filter; >> spice_usb_device_manager_get_type; >> spice_usb_device_manager_is_device_connected; >> +spice_usb_device_matches_interface; >> spice_usb_device_widget_get_type; >> spice_usb_device_widget_new; >> spice_usbredir_channel_get_type; >> diff --git a/gtk/spice-glib-sym-file b/gtk/spice-glib-sym-file >> index 2189fa5..83f7f18 100644 >> --- a/gtk/spice-glib-sym-file >> +++ b/gtk/spice-glib-sym-file >> @@ -101,6 +101,7 @@ spice_usb_device_manager_get_devices >> spice_usb_device_manager_get_devices_with_filter >> spice_usb_device_manager_get_type >> spice_usb_device_manager_is_device_connected >> +spice_usb_device_matches_interface >> spice_usbredir_channel_get_type >> spice_util_get_debug >> spice_util_get_version_string >> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c >> index 5013b6c..6749b0d 100644 >> --- a/gtk/usb-device-manager.c >> +++ b/gtk/usb-device-manager.c >> @@ -706,6 +706,30 @@ static gboolean >> spice_usb_device_manager_get_device_descriptor( >> return TRUE; >> } >> +static gboolean spice_usb_device_manager_get_config_descriptor( >> + libusb_device *libdev, >> + struct libusb_config_descriptor **config) >> +{ >> + int errcode; >> + const gchar *errstr; >> + >> + g_return_val_if_fail(libdev != NULL, FALSE); >> + g_return_val_if_fail(config != NULL, FALSE); >> + >> + errcode = libusb_get_active_config_descriptor(libdev, config); > > > I think you should first look at the device descriptor for that information > and only if it's specified there that the information is to be found > in the interface, look at the config_descriptor. (But I'm not > sure, maybe the config_descriptor always contains that > information) Hmmm. You're right. When bDeviceClass is 0, means that I should check in the Interface level, as shown in the lsusb -v Bus 003 Device 013: ID 05ac:0220 Apple, Inc. Aluminum Keyboard (ANSI) Couldn't open device, some information will be missing Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 8 idVendor 0x05ac Apple, Inc. idProduct 0x0220 Aluminum Keyboard (ANSI) bcdDevice 0.69 iManufacturer 1 iProduct 2 iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 59 bNumInterfaces 2 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xa0 (Bus Powered) Remote Wakeup MaxPower 20mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 1 Boot Interface Subclass bInterfaceProtocol 1 Keyboard iInterface 0 HID Device Descriptor: bLength 9 bDescriptorType 33 bcdHID 1.11 bCountryCode 33 US bNumDescriptors 1 bDescriptorType 34 Report wDescriptorLength 75 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0008 1x 8 bytes bInterval 10 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 0 No Subclass bInterfaceProtocol 0 None iInterface 0 HID Device Descriptor: bLength 9 bDescriptorType 33 bcdHID 1.11 bCountryCode 0 Not supported bNumDescriptors 1 bDescriptorType 34 Report wDescriptorLength 47 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 10 > > >> + if (errcode < 0) { >> + int bus, addr; >> + >> + bus = libusb_get_bus_number(libdev); >> + addr = libusb_get_device_address(libdev); >> + errstr = spice_usbutil_libusb_strerror(errcode); >> + g_warning("Cannot get config descriptor for (%p) %d.%d -- >> %s(%d)", >> + libdev, bus, addr, errstr, errcode); >> + return FALSE; >> + } >> + return TRUE; >> +} >> + >> static gboolean spice_usb_device_manager_get_libdev_vid_pid( >> libusb_device *libdev, int *vid, int *pid) >> { >> @@ -1726,7 +1750,79 @@ gchar >> *spice_usb_device_get_description(SpiceUsbDevice *device, const gchar *for >> #endif >> } >> +static guint8 spice_usb_device_get_interface_class(libusb_device >> *libdev) >> +{ >> + struct libusb_config_descriptor *config; >> + guint8 interface_class; >> + >> + g_return_val_if_fail(libdev != NULL, 0); >> + >> + config = g_malloc(sizeof(*config)); > > > You do not need to allocate memory here. > libusb_get_active_config_descriptor allocates memory for you (if > successful). > So, this memory is leaked. > Same below. Ooops, thanks! > > >> + spice_usb_device_manager_get_config_descriptor(libdev, &config); >> + interface_class = config->interface->altsetting->bInterfaceClass; >> + libusb_free_config_descriptor(config); >> + >> + return interface_class; >> +} >> + >> +static guint8 spice_usb_device_get_interface_subclass(libusb_device >> *libdev) >> +{ >> + struct libusb_config_descriptor *config; >> + guint8 interface_subclass; >> + >> + g_return_val_if_fail(libdev != NULL, 0); >> + >> + config = g_malloc(sizeof(*config)); >> + spice_usb_device_manager_get_config_descriptor(libdev, &config); >> + interface_subclass = >> config->interface->altsetting->bInterfaceSubClass; >> + libusb_free_config_descriptor(config); >> + >> + return interface_subclass; >> +} >> + >> +static guint8 spice_usb_device_get_interface_protocol(libusb_device >> *libdev) >> +{ >> + struct libusb_config_descriptor *config; >> + guint8 interface_protocol; >> + >> + g_return_val_if_fail(libdev != NULL, 0); >> + >> + config = g_malloc(sizeof(*config)); >> + spice_usb_device_manager_get_config_descriptor(libdev, &config); >> + interface_protocol = >> config->interface->altsetting->bInterfaceProtocol; >> + libusb_free_config_descriptor(config); >> + >> + return interface_protocol; >> +} >> + >> +/** >> + * spice_usb_device_matches_interface: >> + * @device: #SpiceUsbDevice to check if the interface info matches with >> + * @class: the interface class to be checked >> + * @subclass: the interface usbclass to be checked > > s/usbclass/subclass/ >> >> + * @protocol; the interface protocol to be checked >> + * >> + * Compares if the device's interface class, subclass and protocol match >> with >> + * the received interface class, subclass and protocol. >> + * >> + * Returns: TRUE if the device's interface matches with the receveid one. >> + * FALSE, otherwise. >> + * >> + * Since: 0.27 >> + **/ >> +gboolean spice_usb_device_matches_interface(const SpiceUsbDevice *device, >> guint8 class, guint8 subclass, guint8 protocol) >> +{ >> + const SpiceUsbDeviceInfo *info = (const SpiceUsbDeviceInfo *)device; >> + >> + g_return_val_if_fail(info != NULL, FALSE); > > > Note that on Windows, currently, we do not keep info->libdev, as > it may change upon device installation. > Yeah, unfortunately Windows is not going to take advantage of these bits. > Regards, > Uri. >> >> + >> + if ((spice_usb_device_get_interface_class(info->libdev) == class) && >> + (spice_usb_device_get_interface_subclass(info->libdev) == >> subclass) && >> + (spice_usb_device_get_interface_protocol(info->libdev) == >> protocol)) >> + return TRUE; >> + return FALSE; >> +} >> #ifdef USE_USBREDIR >> /* >> diff --git a/gtk/usb-device-manager.h b/gtk/usb-device-manager.h >> index a7e3515..9959fe5 100644 >> --- a/gtk/usb-device-manager.h >> +++ b/gtk/usb-device-manager.h >> @@ -89,6 +89,7 @@ GType spice_usb_device_get_type(void); >> GType spice_usb_device_manager_get_type(void); >> gchar *spice_usb_device_get_description(SpiceUsbDevice *device, const >> gchar *format); >> +gboolean spice_usb_device_matches_interface(const SpiceUsbDevice *device, >> guint8 class, guint8 subclass, guint8 protocol); >> SpiceUsbDeviceManager *spice_usb_device_manager_get(SpiceSession >> *session, >> GError **err); > > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel Best Regards, -- Fabiano Fidêncio _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel