[Spice-devel] [spice-gtk Win32 v3 05/12] Make SpiceUsbDevice a gobject, instead of a box for libusb_device

Arnon Gilboa agilboa at redhat.com
Thu Jun 28 04:26:23 PDT 2012


Seems great to me, but few comments below.

Uri Lublin wrote:
> Note that this change may affect performance a bit, as we now need to look
> for the libusb_device if we need it. Likely it's negligible.
> ---
>  gtk/Makefile.am               |    4 +
>  gtk/channel-usbredir-priv.h   |    4 +-
>  gtk/channel-usbredir.c        |   46 +++++-----
>  gtk/map-file                  |    7 ++
>  gtk/usb-device-manager-priv.h |    3 +
>  gtk/usb-device-manager.c      |  194 +++++++++++++++++++++++++++++------------
>  gtk/usb-device-manager.h      |    5 +-
>  gtk/usb-device-widget.c       |    9 +--
>  spice-common                  |    2 +-
>  9 files changed, 182 insertions(+), 92 deletions(-)
>
> diff --git a/gtk/Makefile.am b/gtk/Makefile.am
> index 0327d65..e79abae 100644
> --- a/gtk/Makefile.am
> +++ b/gtk/Makefile.am
> @@ -228,6 +228,9 @@ libspice_client_glib_2_0_la_SOURCES =			\
>  	channel-usbredir-priv.h				\
>  	smartcard-manager.c				\
>  	smartcard-manager-priv.h			\
> +	spice-usb-device.c				\
> +	spice-usb-device.h				\
> +	spice-usb-device-priv.h				\
>  	usb-device-manager.c				\
>  	usb-device-manager-priv.h			\
>  	usbutil.c					\
> @@ -266,6 +269,7 @@ libspice_client_glibinclude_HEADERS =	\
>  	channel-record.h		\
>  	channel-smartcard.h		\
>  	channel-usbredir.h		\
> +	spice-usb-device.h		\
>  	usb-device-manager.h		\
>  	smartcard-manager.h		\
>  	$(NULL)
> diff --git a/gtk/channel-usbredir-priv.h b/gtk/channel-usbredir-priv.h
> index 5d28c79..e6cd538 100644
> --- a/gtk/channel-usbredir-priv.h
> +++ b/gtk/channel-usbredir-priv.h
> @@ -37,7 +37,7 @@ void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel,
>     (through spice_channel_connect()), before calling this. */
>  void spice_usbredir_channel_connect_device_async(
>                                          SpiceUsbredirChannel *channel,
> -                                        libusb_device        *device,
> +                                        SpiceUsbDevice       *device,
>                                          GCancellable         *cancellable,
>                                          GAsyncReadyCallback   callback,
>                                          gpointer              user_data);
> @@ -48,7 +48,7 @@ gboolean spice_usbredir_channel_connect_device_finish(
>
>  void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel);
>
> -libusb_device *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel);
> +SpiceUsbDevice *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel);
>
>  void spice_usbredir_channel_get_guest_filter(
>                            SpiceUsbredirChannel               *channel,
> diff --git a/gtk/channel-usbredir.c b/gtk/channel-usbredir.c
> index 3d57152..63efb77 100644
> --- a/gtk/channel-usbredir.c
> +++ b/gtk/channel-usbredir.c
> @@ -35,6 +35,7 @@
>  #include "spice-client.h"
>  #include "spice-common.h"
>
> +#include "spice-usb-device-priv.h"
>  #include "spice-channel-priv.h"
>  #include "glib-compat.h"
>
> @@ -65,7 +66,7 @@ enum SpiceUsbredirChannelState {
>  };
>
>  struct _SpiceUsbredirChannelPrivate {
> -    libusb_device *device;
> +    SpiceUsbDevice *device;
>      struct usbredirhost *host;
>      /* To catch usbredirhost error messages and report them as a GError */
>      GError **catch_error;
> @@ -211,6 +212,7 @@ static gboolean spice_usbredir_channel_open_device(
>      SpiceUsbredirChannel *channel, GError **err)
>  {
>      SpiceUsbredirChannelPrivate *priv = channel->priv;
> +    SpiceUsbDeviceManager *manager;
>      libusb_device_handle *handle = NULL;
>      int rc, status;
>
> @@ -220,7 +222,11 @@ static gboolean spice_usbredir_channel_open_device(
>  #endif
>                           , FALSE);
>
> -    rc = libusb_open(priv->device, &handle);
> +    manager = spice_usb_device_manager_get(
> +                    spice_channel_get_session(SPICE_CHANNEL(channel)), err);
> +    if (!manager)
> +        return FALSE;
> +    rc = spice_usb_device_libusb_open(manager, priv->device, &handle);
>      if (rc != 0) {
>          g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>                      "Could not open usb device: %s [%i]",
> @@ -236,10 +242,7 @@ static gboolean spice_usbredir_channel_open_device(
>          return FALSE;
>      }
>
> -    if (!spice_usb_device_manager_start_event_listening(
> -            spice_usb_device_manager_get(
> -                spice_channel_get_session(SPICE_CHANNEL(channel)), NULL),
> -            err)) {
> +    if (!spice_usb_device_manager_start_event_listening(manager, err)) {
>          usbredirhost_set_device(priv->host, NULL);
>          return FALSE;
>      }
> @@ -272,7 +275,7 @@ static void spice_usbredir_channel_open_acl_cb(
>      }
>      if (err) {
>          g_simple_async_result_take_error(priv->result, err);
> -        libusb_unref_device(priv->device);
> +        g_object_unref(priv->device);
>          priv->device = NULL;
>          priv->state  = STATE_DISCONNECTED;
>      }
> @@ -290,7 +293,7 @@ static void spice_usbredir_channel_open_acl_cb(
>  G_GNUC_INTERNAL
>  void spice_usbredir_channel_connect_device_async(
>                                            SpiceUsbredirChannel *channel,
> -                                          libusb_device        *device,
> +                                          SpiceUsbDevice       *device,
>                                            GCancellable         *cancellable,
>                                            GAsyncReadyCallback   callback,
>                                            gpointer              user_data)
> @@ -302,7 +305,7 @@ void spice_usbredir_channel_connect_device_async(
>  #endif
>
>      g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
> -    g_return_if_fail(device != NULL);
> +    g_return_if_fail(SPICE_IS_USB_DEVICE(device));
>
>      SPICE_DEBUG("connecting usb channel %p", channel);
>
> @@ -323,7 +326,7 @@ void spice_usbredir_channel_connect_device_async(
>          goto done;
>      }
>
> -    priv->device = libusb_ref_device(device);
> +    priv->device = g_object_ref(device);
>  #if USE_POLKIT
>      priv->result = result;
>      priv->state  = STATE_WAITING_FOR_ACL_HELPER;
> @@ -331,8 +334,8 @@ void spice_usbredir_channel_connect_device_async(
>      g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),
>                   "inhibit-keyboard-grab", TRUE, NULL);
>      spice_usb_acl_helper_open_acl(priv->acl_helper,
> -                                  libusb_get_bus_number(device),
> -                                  libusb_get_device_address(device),
> +                                  spice_usb_device_get_busnum(device),
> +                                  spice_usb_device_get_devaddr(device),
>                                    cancellable,
>                                    spice_usbredir_channel_open_acl_cb,
>                                    channel);
> @@ -340,7 +343,7 @@ void spice_usbredir_channel_connect_device_async(
>  #else
>      if (!spice_usbredir_channel_open_device(channel, &err)) {
>          g_simple_async_result_take_error(result, err);
> -        libusb_unref_device(priv->device);
> +        g_object_unref(priv->device);
>          priv->device = NULL;
>      }
>  #endif
> @@ -398,7 +401,7 @@ void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel)
>                  spice_channel_get_session(SPICE_CHANNEL(channel)), NULL));
>          /* This also closes the libusb handle we passed from open_device */
>          usbredirhost_set_device(priv->host, NULL);
> -        libusb_unref_device(priv->device);
> +        g_object_unref(priv->device);
>          priv->device = NULL;
>          priv->state  = STATE_DISCONNECTED;
>          break;
> @@ -406,7 +409,7 @@ void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel)
>  }
>
>  G_GNUC_INTERNAL
> -libusb_device *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel)
> +SpiceUsbDevice *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel)
>  {
>      return channel->priv->device;
>  }
> @@ -550,7 +553,7 @@ enum {
>  };
>
>  struct DEVICE_ERROR {
> -    libusb_device *device;
> +    SpiceUsbDevice *device;
>      GError *error;
>  };
>
> @@ -569,7 +572,7 @@ static void do_emit_main_context(GObject *object, int event, gpointer params)
>              spice_usb_device_manager_device_error(
>                  spice_usb_device_manager_get(
>                      spice_channel_get_session(SPICE_CHANNEL(channel)), NULL),
> -                (SpiceUsbDevice *)p->device, p->error);
> +                p->device, p->error);
>          }
>          break;
>      }
> @@ -624,14 +627,13 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)
>
>      r = usbredirhost_read_guest_data(priv->host);
>      if (r != 0) {
> -        libusb_device *device = priv->device;
> +        SpiceUsbDevice *device = priv->device;
>          gchar *desc;
>          GError *err;
>
>          g_return_if_fail(device != NULL);
>
> -        desc = spice_usb_device_get_description((SpiceUsbDevice *)device,
> -                                                NULL);
> +        desc = spice_usb_device_get_description(device, NULL);
>          switch (r) {
>          case usbredirhost_read_parse_error:
>              err = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> @@ -655,9 +657,9 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)
>
>          SPICE_DEBUG("%s", err->message);
>
> -        g_boxed_copy(spice_usb_device_get_type(), device);
> +        g_object_ref(device);
>          emit_main_context(channel, DEVICE_ERROR, device, err);
> -        g_boxed_free(spice_usb_device_get_type(), device);
> +        g_object_unref(device);
>
>          g_error_free(err);
>      }
> diff --git a/gtk/map-file b/gtk/map-file
> index 32ead37..7f18d19 100644
> --- a/gtk/map-file
> +++ b/gtk/map-file
> @@ -100,6 +100,13 @@ spice_usbredir_channel_get_type;
>  spice_util_get_debug;
>  spice_util_get_version_string;
>  spice_util_set_debug;
> +spice_usb_device_get_type;
> +spice_usb_device_new;
> +spice_usb_device_set_info;
> +spice_usb_device_get_busnum;
> +spice_usb_device_get_devaddr;
> +spice_usb_device_get_vid;
> +spice_usb_device_get_pid;
>   
do we need these lines?
>  local:
>  *;
>  };
> diff --git a/gtk/usb-device-manager-priv.h b/gtk/usb-device-manager-priv.h
> index 912e3bf..6da890c 100644
> --- a/gtk/usb-device-manager-priv.h
> +++ b/gtk/usb-device-manager-priv.h
> @@ -34,6 +34,9 @@ void spice_usb_device_manager_stop_event_listening(
>  void spice_usb_device_manager_device_error(
>      SpiceUsbDeviceManager *manager, SpiceUsbDevice *device, GError *err);
>
> +int spice_usb_device_libusb_open(SpiceUsbDeviceManager *self,
> +                                 SpiceUsbDevice *device,
> +                                 struct libusb_device_handle **handle);
>  G_END_DECLS
>
>  #endif /* __SPICE_USB_DEVICE_MANAGER_PRIV_H__ */
> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
> index 25cb14a..4448a74 100644
> --- a/gtk/usb-device-manager.c
> +++ b/gtk/usb-device-manager.c
> @@ -38,6 +38,8 @@
>  #include "spice-client.h"
>  #include "spice-marshal.h"
>  #include "usb-device-manager-priv.h"
> +#include "spice-usb-device.h"
> +#include "spice-usb-device-priv.h"
>
>  #include <glib/gi18n.h>
>
> @@ -112,14 +114,11 @@ static void spice_usb_device_manager_uevent_cb(GUdevClient     *client,
>                                                 gpointer         user_data);
>  static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
>                                               GUdevDevice            *udev);
> +static
> +libusb_device *spice_usb_device_find_libusb_device(SpiceUsbDeviceManager *self,
> +                                                   SpiceUsbDevice *device);
> +#endif /* ifdef USBREDIR */
>
> -G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device,
> -                    (GBoxedCopyFunc)libusb_ref_device,
> -                    (GBoxedFreeFunc)libusb_unref_device)
> -
> -#else
> -G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device, g_object_ref, g_object_unref)
> -#endif
>  static void spice_usb_device_manager_initable_iface_init(GInitableIface *iface);
>
>  static guint signals[LAST_SIGNAL] = { 0, };
> @@ -127,6 +126,8 @@ static guint signals[LAST_SIGNAL] = { 0, };
>  G_DEFINE_TYPE_WITH_CODE(SpiceUsbDeviceManager, spice_usb_device_manager, G_TYPE_OBJECT,
>       G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE, spice_usb_device_manager_initable_iface_init));
>
> +
> +
>   
nls
>  static void spice_usb_device_manager_init(SpiceUsbDeviceManager *self)
>  {
>      SpiceUsbDeviceManagerPrivate *priv;
> @@ -137,7 +138,7 @@ static void spice_usb_device_manager_init(SpiceUsbDeviceManager *self)
>      priv->channels = g_ptr_array_new();
>  #ifdef USE_USBREDIR
>      priv->devices  = g_ptr_array_new_with_free_func((GDestroyNotify)
> -                                                    libusb_unref_device);
> +                                                    g_object_unref);
>  #endif
>  }
>
> @@ -395,7 +396,7 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
>                       g_cclosure_marshal_VOID__BOXED,
>                       G_TYPE_NONE,
>                       1,
> -                     SPICE_TYPE_USB_DEVICE);
> +                     G_TYPE_POINTER);
>   
why are these changes needed?
>      /**
>       * SpiceUsbDeviceManager::device-removed:
> @@ -414,7 +415,7 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
>                       g_cclosure_marshal_VOID__BOXED,
>                       G_TYPE_NONE,
>                       1,
> -                     SPICE_TYPE_USB_DEVICE);
> +                     G_TYPE_POINTER);
>
>      /**
>       * SpiceUsbDeviceManager::auto-connect-failed:
> @@ -435,7 +436,7 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
>                       g_cclosure_user_marshal_VOID__BOXED_BOXED,
>                       G_TYPE_NONE,
>                       2,
> -                     SPICE_TYPE_USB_DEVICE,
> +                     G_TYPE_POINTER,
>                       G_TYPE_ERROR);
>
>      /**
> @@ -457,7 +458,7 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
>                       g_cclosure_user_marshal_VOID__BOXED_BOXED,
>                       G_TYPE_NONE,
>                       2,
> -                     SPICE_TYPE_USB_DEVICE,
> +                     G_TYPE_POINTER,
>                       G_TYPE_ERROR);
>
>      g_type_class_add_private(klass, sizeof(SpiceUsbDeviceManagerPrivate));
> @@ -515,12 +516,12 @@ static void spice_usb_device_manager_auto_connect_cb(GObject      *gobject,
>                                                       gpointer      user_data)
>  {
>      SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(gobject);
> -    libusb_device *device = user_data;
> +    SpiceUsbDevice *device = user_data;
>      GError *err = NULL;
>
>      spice_usb_device_manager_connect_device_finish(self, res, &err);
>      if (err) {
> -        gchar *desc = spice_usb_device_get_description((SpiceUsbDevice *)device, NULL);
> +        gchar *desc = spice_usb_device_get_description(device, NULL);
>          g_prefix_error(&err, "Could not auto-redirect %s: ", desc);
>          g_free(desc);
>
> @@ -528,16 +529,18 @@ static void spice_usb_device_manager_auto_connect_cb(GObject      *gobject,
>          g_signal_emit(self, signals[AUTO_CONNECT_FAILED], 0, device, err);
>          g_error_free(err);
>      }
> -    libusb_unref_device(device);
> +    g_object_unref(device);
>  }
>
>  static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
>                                               GUdevDevice            *udev)
>  {
>      SpiceUsbDeviceManagerPrivate *priv = self->priv;
> -    libusb_device *device = NULL, **dev_list = NULL;
> +    libusb_device **dev_list = NULL;
> +    SpiceUsbDevice *device = NULL;
>      const gchar *devtype, *devclass;
>      int i, bus, address;
> +    gboolean filter_ok = FALSE;
>
>      devtype = g_udev_device_get_property(udev, "DEVTYPE");
>      /* Check if this is a usb device (and not an interface) */
> @@ -562,7 +565,12 @@ static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
>      for (i = 0; dev_list && dev_list[i]; i++) {
>          if (libusb_get_bus_number(dev_list[i]) == bus &&
>              libusb_get_device_address(dev_list[i]) == address) {
> -            device = libusb_ref_device(dev_list[i]);
> +            device = spice_usb_device_new();
> +            spice_usb_device_set_info(device, dev_list[i]);
> +            filter_ok = usbredirhost_check_device_filter(
> +                            priv->auto_conn_filter_rules,
> +                            priv->auto_conn_filter_rules_count,
> +                            dev_list[i], 0) == 0;
>              break;
>          }
>      }
> @@ -579,21 +587,17 @@ static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
>      g_ptr_array_add(priv->devices, device);
>
>      if (priv->auto_connect) {
> -        gboolean can_redirect, auto_ok;
> +        gboolean can_redirect;
>
>          can_redirect = spice_usb_device_manager_can_redirect_device(
> -                                        self, (SpiceUsbDevice *)device, NULL);
> -
> -        auto_ok = usbredirhost_check_device_filter(
> -                            priv->auto_conn_filter_rules,
> -                            priv->auto_conn_filter_rules_count,
> -                            device, 0) == 0;
> +                                        self, device, NULL);
>
> -        if (can_redirect && auto_ok)
> +        if (can_redirect && filter_ok) {
>              spice_usb_device_manager_connect_device_async(self,
> -                                   (SpiceUsbDevice *)device, NULL,
> +                                   device, NULL,
>                                     spice_usb_device_manager_auto_connect_cb,
> -                                   libusb_ref_device(device));
> +                                   g_object_ref(device));
> +        }
>      }
>
>      SPICE_DEBUG("device added %p", device);
> @@ -604,7 +608,7 @@ static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager  *self,
>                                                  GUdevDevice            *udev)
>  {
>      SpiceUsbDeviceManagerPrivate *priv = self->priv;
> -    libusb_device *curr, *device = NULL;
> +    SpiceUsbDevice *curr, *device = NULL;
>      int bus, address;
>      guint i;
>
> @@ -613,8 +617,8 @@ static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager  *self,
>
>      for (i = 0; i < priv->devices->len; i++) {
>          curr = g_ptr_array_index(priv->devices, i);
> -        if (libusb_get_bus_number(curr) == bus &&
> -               libusb_get_device_address(curr) == address) {
> +        if (spice_usb_device_get_busnum(curr) == bus &&
> +               spice_usb_device_get_devaddr(curr) == address) {
>              device = curr;
>              break;
>          }
> @@ -727,11 +731,10 @@ void spice_usb_device_manager_device_error(
>  #endif
>
>  static SpiceUsbredirChannel *spice_usb_device_manager_get_channel_for_dev(
> -    SpiceUsbDeviceManager *manager, SpiceUsbDevice *_device)
> +    SpiceUsbDeviceManager *manager, SpiceUsbDevice *device)
>  {
>  #ifdef USE_USBREDIR
>      SpiceUsbDeviceManagerPrivate *priv = manager->priv;
> -    libusb_device *device = (libusb_device *)_device;
>      guint i;
>
>      for (i = 0; i < priv->channels->len; i++) {
> @@ -796,10 +799,10 @@ GPtrArray* spice_usb_device_manager_get_devices(SpiceUsbDeviceManager *self)
>      guint i;
>
>      devices_copy = g_ptr_array_new_with_free_func((GDestroyNotify)
> -                                                  libusb_unref_device);
> +                                                  g_object_unref);
>      for (i = 0; i < priv->devices->len; i++) {
> -        libusb_device *device = g_ptr_array_index(priv->devices, i);
> -        g_ptr_array_add(devices_copy, libusb_ref_device(device));
> +        SpiceUsbDevice *device = g_ptr_array_index(priv->devices, i);
> +        g_ptr_array_add(devices_copy, g_object_ref(device));
>      }
>  #endif
>
> @@ -864,7 +867,7 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
>              continue; /* Skip already used channels */
>
>          spice_usbredir_channel_connect_device_async(channel,
> -                                 (libusb_device *)device,
> +                                 device,
>                                   cancellable,
>                                   spice_usb_device_manager_channel_connect_cb,
>                                   result);
> @@ -959,13 +962,20 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>          g_ptr_array_index(priv->channels, 0),
>          &guest_filter_rules, &guest_filter_rules_count);
>
> -    if (guest_filter_rules &&
> -            usbredirhost_check_device_filter(
> +    if (guest_filter_rules) {
> +        gboolean filter_ok;
> +        libusb_device *ldev;
> +        ldev = spice_usb_device_find_libusb_device(self, device);
> +        g_return_val_if_fail(ldev != NULL, FALSE);
> +        filter_ok = (usbredirhost_check_device_filter(
>                              guest_filter_rules, guest_filter_rules_count,
> -                            (libusb_device *)device, 0) != 0) {
> -        g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> -                            _("Some USB devices are blocked by host policy"));
> -        return FALSE;
> +                            ldev, 0) == 0);
> +        libusb_unref_device(ldev);
> +        if (!filter_ok) {
> +            g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> +                                _("Some USB devices are blocked by host policy"));
> +            return FALSE;
> +        }
>      }
>
>      /* Check if there are free channels */
> @@ -1009,34 +1019,32 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>   *
>   * Returns: a newly-allocated string holding the description, or %NULL if failed
>   */
> -gchar *spice_usb_device_get_description(SpiceUsbDevice *_device, const gchar *format)
> +gchar *spice_usb_device_get_description(SpiceUsbDevice *device, const gchar *format)
>  {
>  #ifdef USE_USBREDIR
> -    libusb_device *device = (libusb_device *)_device;
> -    struct libusb_device_descriptor desc;
> -    int bus, address;
> +    int bus, addr, vid, pid;
>      gchar *description, *descriptor, *manufacturer = NULL, *product = NULL;
>
> -    g_return_val_if_fail(device != NULL, NULL);
> +    g_return_val_if_fail(SPICE_IS_USB_DEVICE(device), NULL);
>
> -    bus     = libusb_get_bus_number(device);
> -    address = libusb_get_device_address(device);
> +    bus  = spice_usb_device_get_busnum(device);
> +    addr = spice_usb_device_get_devaddr(device);
> +    vid  = spice_usb_device_get_vid(device);
> +    pid  = spice_usb_device_get_pid(device);
>
> -    if (libusb_get_device_descriptor(device, &desc) == LIBUSB_SUCCESS) {
> -        spice_usb_util_get_device_strings(bus, address,
> -                                          desc.idVendor, desc.idProduct,
> -                                          &manufacturer, &product);
> -        descriptor = g_strdup_printf("[%04x:%04x]", desc.idVendor, desc.idProduct);
> +    if ((vid > 0) && (pid > 0)) {
> +        descriptor = g_strdup_printf("[%04x:%04x]", vid, pid);
>      } else {
> -        spice_usb_util_get_device_strings(bus, address, -1, -1,
> -                                          &manufacturer, &product);
>          descriptor = g_strdup("");
>      }
>
> +    spice_usb_util_get_device_strings(bus, addr, vid, pid,
> +                                      &manufacturer, &product);
> +
>      if (!format)
>          format = _("%s %s %s at %d-%d");
>
> -    description = g_strdup_printf(format, manufacturer, product, descriptor, bus, address);
> +    description = g_strdup_printf(format, manufacturer, product, descriptor, bus, addr);
>
>      g_free(manufacturer);
>      g_free(descriptor);
> @@ -1047,3 +1055,75 @@ gchar *spice_usb_device_get_description(SpiceUsbDevice *_device, const gchar *fo
>      return NULL;
>  #endif
>  }
> +
> +
> +
>   
nls
> +/*
> + * Caller must libusb_unref_device the libusb_device returned by this function.
> + * Returns a libusb_device, or NULL upon failure
> + */
> +static
> +libusb_device *spice_usb_device_find_libusb_device(SpiceUsbDeviceManager *self,
> +                                                   SpiceUsbDevice *device)
> +{
> +    libusb_device *d, **devlist;
> +    guint8 bus, addr;
> +    int i;
> +
> +    g_return_val_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self), NULL);
> +    g_return_val_if_fail(SPICE_IS_USB_DEVICE(device), NULL);
> +    g_return_val_if_fail(self->priv != NULL, NULL);
> +    g_return_val_if_fail(self->priv->context != NULL, NULL);
> +
> +    bus  = spice_usb_device_get_busnum(device);
> +    addr = spice_usb_device_get_devaddr(device);
> +
> +    libusb_get_device_list(self->priv->context, &devlist);
> +    if (!devlist)
> +        return NULL;
> +
> +    for (i = 0; (d = devlist[i]) != NULL; i++) {
> +        if ((libusb_get_bus_number(d) == bus) &&
> +            (libusb_get_device_address(d) == addr)) {
> +            libusb_ref_device(d);
> +            break;
> +        }
> +    }
> +
> +    libusb_free_device_list(devlist, 1);
> +
> +    return d;
> +}
> +
> +int spice_usb_device_libusb_open(SpiceUsbDeviceManager *self,
> +                                 SpiceUsbDevice *device,
> +                                 struct libusb_device_handle **handle)
> +{
> +    libusb_device *ldev;
> +    guint8 bus, addr;
> +    int rc;
> +
> +    g_return_val_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self), -1);
> +    g_return_val_if_fail(SPICE_IS_USB_DEVICE(device), -2);
> +
> +    bus  = spice_usb_device_get_busnum(device);
> +    addr = spice_usb_device_get_devaddr(device);
> +
> +    ldev = spice_usb_device_find_libusb_device(self, device);
> +    if (ldev) {
> +        rc = libusb_open(ldev, handle);
> +        if (rc) {
> +            const gchar *errstr = spice_usbutil_libusb_strerror(rc);
> +            g_warning("Failed to open usb device %d.%d -- %s (%i)",
> +                      bus, addr, errstr, rc);
> +        }
> +        libusb_unref_device(ldev);
> +    } else {
> +        rc = -3;
>   
doesn't your rc values collide with libusb_error returned by libusb_open?
> +        g_warning("Did not find device %d.%d to open", bus, addr);
> +    }
> +
> +
> +
> +
nls
>     return rc;
> +}
> diff --git a/gtk/usb-device-manager.h b/gtk/usb-device-manager.h
> index df138ee..e5577d8 100644
> --- a/gtk/usb-device-manager.h
> +++ b/gtk/usb-device-manager.h
> @@ -23,6 +23,7 @@
>
>  #include "spice-client.h"
>  #include <gio/gio.h>
> +#include "spice-usb-device.h"
>
>  G_BEGIN_DECLS
>
> @@ -33,13 +34,12 @@ G_BEGIN_DECLS
>  #define SPICE_IS_USB_DEVICE_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), SPICE_TYPE_USB_DEVICE_MANAGER))
>  #define SPICE_USB_DEVICE_MANAGER_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), SPICE_TYPE_USB_DEVICE_MANAGER, SpiceUsbDeviceManagerClass))
>
> -#define SPICE_TYPE_USB_DEVICE                    (spice_usb_device_get_type())
> +
>   
nl
>  typedef struct _SpiceUsbDeviceManager SpiceUsbDeviceManager;
>  typedef struct _SpiceUsbDeviceManagerClass SpiceUsbDeviceManagerClass;
>  typedef struct _SpiceUsbDeviceManagerPrivate SpiceUsbDeviceManagerPrivate;
>
> -typedef struct _SpiceUsbDevice SpiceUsbDevice;
>   
nl
>  /**
>   * SpiceUsbDeviceManager:
> @@ -85,7 +85,6 @@ struct _SpiceUsbDeviceManagerClass
>      gchar _spice_reserved[SPICE_RESERVED_PADDING];
>  };
>
> -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);
> diff --git a/gtk/usb-device-widget.c b/gtk/usb-device-widget.c
> index 3ed81e4..c342981 100644
> --- a/gtk/usb-device-widget.c
> +++ b/gtk/usb-device-widget.c
> @@ -465,11 +465,6 @@ static void checkbox_clicked_cb(GtkWidget *check, gpointer user_data)
>      spice_usb_device_widget_update_status(self);
>  }
>
> -static void checkbox_usb_device_destroy_notify(gpointer data)
> -{
> -    g_boxed_free(spice_usb_device_get_type(), data);
> -}
> -
>  static void device_added_cb(SpiceUsbDeviceManager *manager,
>      SpiceUsbDevice *device, gpointer user_data)
>  {
> @@ -489,8 +484,8 @@ static void device_added_cb(SpiceUsbDeviceManager *manager,
>
>      g_object_set_data_full(
>              G_OBJECT(check), "usb-device",
> -            g_boxed_copy(spice_usb_device_get_type(), device),
> -            checkbox_usb_device_destroy_notify);
> +            g_object_ref(device),
> +            g_object_unref);
>      g_signal_connect(G_OBJECT(check), "clicked",
>                       G_CALLBACK(checkbox_clicked_cb), self);
>
> diff --git a/spice-common b/spice-common
> index 5f44094..22fc0b0 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit 5f4409494066b5f59df58d6207fdbb0441aa9e90
> +Subproject commit 22fc0b0145876b90385c1c88923bcd72a6380812
>   



More information about the Spice-devel mailing list