On 11/12/2014 07:22 PM, Fabiano Fidêncio wrote:
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.
The default can be to block all HID devices.
If a user wants to usbredir a specific device, e.g. a Joystick,
s/he can find out its VID/PID (using lsusb) and add a rule
to the usb-filter accordingly.
I agree it's not as convenient as filtering by subclass/protocol.
Another comment with regards to your patch is:
config->interface may hold an array of (bNumInterfaces) interfaces
and you are only looking at the first one.
Same for config->interface->altsetting
As for the Windows comment, with the patch applied windows
build is going to fail, isn't it ?
Regards,
Uri.
---
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,
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel