subject:"\[Spice\-devel\] \[spice\-gtk\] \[PATCH\] Provide a method to check if a USB device matches a specific class, subclass and protocol"

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

2014-11-12 Thread Uri Lublin


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

---
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)


+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.


+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;
+}
+
+/**
+ *

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

2014-11-12 Thread Fabiano Fidêncio

Hi Uri,

On Wed, Nov 12, 2014 at 6:01 PM, Uri Lublin u...@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:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0
  bDeviceProtocol 0
  bMaxPacketSize0 8
  idVendor   0x05ac Apple, Inc.
  idProduct  0x0220 Aluminum Keyboard (ANSI)
  bcdDevice0.69
  iManufacturer   1
  iProduct2
  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
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 3 Human Interface Device
  bInterfaceSubClass  1 Boot Interface Subclass
  bInterfaceProtocol  1 Keyboard
  iInterface  0
HID Device Descriptor:
  bLength 9
  bDescriptorType33
  bcdHID   1.11
  bCountryCode   33 US
  bNumDescriptors 1
  bDescriptorType34 Report
  wDescriptorLength  75
 Report Descriptors:
   ** UNAVAILABLE **
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

2014-11-11 Thread Cody Chan

Great!
Is there a good way to let it work for windows client?
libusb is not fully support for windows...
I tried to build libwdi with WDK, but I don't think it's a good way.

On Tue, Nov 11, 2014 at 10:29 PM, Fabiano Fidêncio fiden...@redhat.com
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.
 ---
 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);
 +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));
 +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
 + * @protocol; the interface protocol to be checked
 + *
 + * Compares if the device's interface class, subclass

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

2014-11-11 Thread Zeeshan Ali (Khattak)

On Tue, Nov 11, 2014 at 2:29 PM, Fabiano Fidêncio fiden...@redhat.com 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.

I don't think its a good idea to create a separate API for this on the
device level. I suggest either or both of following:

* Extend (if possible without breaking existing API) the existing
filtering mechanism for DeviceManager for this.
* Provide getters for all these properties on device level so Apps can
use that info for different things, including of course to filter
devices in a list.

 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 :-)

After all that practice with this in Boxes, I'm pretty sure you can do
much better if you just try a bit.

-- 
Regards,

Zeeshan Ali (Khattak)

Befriend GNOME: http://www.gnome.org/friends/
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

2014-11-11 Thread Marc-André Lureau

Hi

On Tue, Nov 11, 2014 at 3:29 PM, Fabiano Fidêncio fiden...@redhat.com
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.
 ---
 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 :-)


Not so much the log, but rather the code could have been made much shorter.

I would rather see a spice_usb_device_get_libusb_device() getter tbh. I
don't think libusb is going to be replaced any time soon.

Hans, could you review too? thanks


---
  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);
 +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));
 +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
 + * @protocol; the interface protocol

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

Re: [Spice-devel] [spice-gtk] [PATCH] Provide a method to check if a USB device matches a specific class, subclass and protocol

5 matches

Site Navigation

Mail list logo

Footer information