[Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol
Fabiano Fidêncio
fabiano at fidencio.org
Wed Nov 12 09:22:58 PST 2014
Hi Uri,
On Wed, Nov 12, 2014 at 6:01 PM, Uri Lublin <uril at redhat.com> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
Best Regards,
--
Fabiano Fidêncio
More information about the Spice-devel
mailing list