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

[Uri Lublin]

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.