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

[Christophe Fergeau]

On Thu, Jun 28, 2012 at 04:46:34AM +0300, 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(-)
> 
> @@ -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 you not keeping SPICE_TYPE_USB_DEVICE here and in the calls below?

> 
>      /**
>       * 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));

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

I don't think this will do the right thing in error cases, get_vid returns
an uint16_t which value is 0xffff when there's an error, it will be turned
into 65535 when it's cast to int, it won't be -1. I haven't looked at the
rest of the patch for similar issues.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20120628/9bc29049/attachment.pgp>