[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately



On 05/03/2012 03:21 PM, Eric Blake wrote:
> On 05/03/2012 04:51 AM, Guannan Ren wrote:
>> usbFindDevice():get usb device according to
>>                 idVendor, idProduct, bus, device
>>                 it is the exact match of the four parameters
>>
>> usbFindDeviceByBus():get usb device according to bus, device
>>                   it returns only one usb device same as usbFindDevice
>>
>> usbFindDeviceByVendor():get usb device according to idVendor,idProduct
>>                      it probably returns multiple usb devices.
>>
>> usbDeviceSearch(): a helper function to do the actual search
>> ---
>>  src/libvirt_private.syms |    2 +
>>  src/util/hostusb.c       |  209 +++++++++++++++++++++++++++++++++-------------
>>  src/util/hostusb.h       |   22 ++++--
>>  3 files changed, 170 insertions(+), 63 deletions(-)

This patch doesn't even compile!

qemu/qemu_hostdev.c: In function 'qemuPrepareHostdevUSBDevices':
qemu/qemu_hostdev.c:599:33: error: too few arguments to function
'usbFindDevice'
../src/util/hostusb.h:40:13: note: declared here

You can't change the signature of usbFindDevice unless you update all
callers in the same patch.

By the way, here are some suggestions I have for your patch:

diff --git i/src/util/hostusb.c w/src/util/hostusb.c
index cad0a6c..533b9c7 100644
--- i/src/util/hostusb.c
+++ w/src/util/hostusb.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2009-2012 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -65,9 +65,10 @@ struct _usbDeviceList {
 };

 typedef enum {
-    USB_DEVICE_ALL = 0,
-    USB_DEVICE_FIND_BY_VENDOR = 1,
-    USB_DEVICE_FIND_BY_BUS = 2,
+    USB_DEVICE_ALL            = 0,      /* no filtering */
+    USB_DEVICE_FIND_BY_VENDOR = 1 << 0, /* filter to vendor matches */
+    USB_DEVICE_FIND_BY_BUS    = 1 << 1, /* filter to bus matches */
+    USB_DEVICE_FIND_ONE       = 1 << 2, /* filter to first match */
 } usbDeviceFindFlags;

 static int usbSysReadFile(const char *f_name, const char *d_name,
@@ -108,7 +109,6 @@ usbDeviceSearch(unsigned int vendor,
                 unsigned int flags)
 {
     DIR *dir = NULL;
-    bool found = false;
     char *ignore = NULL;
     struct dirent *de;
     usbDeviceList *list = NULL, *ret = NULL;
@@ -154,29 +154,13 @@ usbDeviceSearch(unsigned int vendor,
                            10, &found_devno) < 0)
             goto cleanup;

-        /*
-         * Don't set found to true in order to continue the loop
-         * to find multiple USB devices with same idVendor and idProduct
-         */
-        if (flags && !(flags & ~USB_DEVICE_FIND_BY_VENDOR))
-            if (found_prod != product || found_vend != vendor)
-                continue;
-
-        if (flags && !(flags & ~USB_DEVICE_FIND_BY_BUS)) {
-            if (found_bus != bus || found_devno != devno)
-                continue;
-            found = true;
-        }
+        if ((flags & USB_DEVICE_FIND_BY_VENDOR) &&
+            (found_prod != product || found_vend != vendor))
+            continue;

-        if ((flags & USB_DEVICE_FIND_BY_BUS)
-             && (flags & USB_DEVICE_FIND_BY_VENDOR)) {
-            if (found_prod != product ||
-                found_vend != vendor  ||
-                found_bus != bus ||
-                found_devno != devno)
-                continue;
-            found = true;
-        }
+        if ((flags & USB_DEVICE_FIND_BY_BUS) &&
+            (found_bus != bus || found_devno != devno))
+            continue;

         usb = usbGetDevice(found_bus, found_devno);
         if (!usb)
@@ -187,7 +171,7 @@ usbDeviceSearch(unsigned int vendor,
             goto cleanup;
         }

-        if (found)
+        if (flags & USB_DEVICE_FIND_ONE)
             break;
     }
     ret = list;
@@ -195,7 +179,7 @@ usbDeviceSearch(unsigned int vendor,
 cleanup:
     if (dir) {
         int saved_errno = errno;
-        closedir (dir);
+        closedir(dir);
         errno = saved_errno;
     }

@@ -209,7 +193,8 @@ usbFindDeviceByVendor(unsigned int vendor, unsigned
product)
 {

     usbDeviceList *list;
-    if (!(list = usbDeviceSearch(vendor, product, 0 , 0,
USB_DEVICE_FIND_BY_VENDOR)))
+    if (!(list = usbDeviceSearch(vendor, product, 0 , 0,
+                                 USB_DEVICE_FIND_BY_VENDOR)))
         return NULL;

     if (list->count == 0) {
@@ -228,12 +213,14 @@ usbFindDeviceByBus(unsigned int bus, unsigned devno)
     usbDevice *usb;
     usbDeviceList *list;

-    if (!(list = usbDeviceSearch(0, 0, bus, devno,
USB_DEVICE_FIND_BY_BUS)))
+    if (!(list = usbDeviceSearch(0, 0, bus, devno,
+                                 USB_DEVICE_FIND_BY_BUS |
USB_DEVICE_FIND_ONE)))
         return NULL;

     if (list->count == 0) {
         usbReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Did not find USB device bus:%u device:%u"),
bus, devno);
+                       _("Did not find USB device bus:%u device:%u"),
+                       bus, devno);
         usbDeviceListFree(list);
         return NULL;
     }
@@ -254,7 +241,9 @@ usbFindDevice(unsigned int vendor,
     usbDevice *usb;
     usbDeviceList *list;

-    unsigned int flags = USB_DEVICE_FIND_BY_VENDOR|USB_DEVICE_FIND_BY_BUS;
+    unsigned int flags = (USB_DEVICE_FIND_BY_VENDOR |
+                          USB_DEVICE_FIND_BY_BUS |
+                          USB_DEVICE_FIND_ONE);
     if (!(list = usbDeviceSearch(vendor, product, bus, devno, flags)))
         return NULL;



-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]