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

[Uri Lublin <uril@xxxxxxxxxx>]

On 06/28/2012 01:09 PM, Christophe Fergeau wrote:
> 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?

I'll put them back.

> >       /**
> >        * 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.

I'll change vid, pid values to 0 on error.


Thanks,
    Uri.

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel