Re: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately
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(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 025816a..b9f9015 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1084,6 +1084,8 @@ usbDeviceListGet;
usbDeviceListNew;
usbDeviceListSteal;
usbDeviceSetUsedBy;
+usbFindDeviceByBus;
+usbFindDeviceByVendor;
usbFindDevice;
Nit: Prefix sorts before the longer name.
+
+ /*
+ * 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))
Huh? Read literally, that says:
If I requested filtering (flags is nonzero), and the filtering that I
requested was any bit other than BY_VENDOR, then filter by vendor.
But that's not what you want.
+ if (found_prod != product || found_vend != vendor)
+ continue;
This if condition is filtering - it says, if my vendor doesn't match,
then resume the loop on the next element.
So you _really_ want:
if ((flags & USB_DEVICE_FIND_BY_VENDOR)) &&
(found_prod != product || found_vend != vendor))
continue;
+
+ if (flags && !(flags & ~USB_DEVICE_FIND_BY_BUS)) {
+ if (found_bus != bus || found_devno != devno)
+ continue;
Likewise, this should be:
if ((flags & USB_DEVICE_FIND_BY_BUS) &&
(found_bus != bus || found_devno != devno))
continue;
+ found = true;
+ }
At which point, if you get this far in one iteration of the loop, none
of your filtering requests rejected the current device, so you can set
found to true.
>
> - if (STRPREFIX(de->d_name, "usb"))
> - tmpstr += 3;
> + 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;
+ }
And you don't need this clause at all, because you already took care of
the filtering up above.
+usbDevice *
+usbFindDevice(unsigned int vendor,
+ unsigned int product,
+ unsigned int bus,
+ unsigned int devno)
+{
+ usbDevice *usb;
+ usbDeviceList *list;
+
+ unsigned int flags = USB_DEVICE_FIND_BY_VENDOR|USB_DEVICE_FIND_BY_BUS;
+ if (!(list = usbDeviceSearch(vendor, product, bus, devno, flags)))
This code is then correct if you fix usbDeviceSearch.
+
+usbDevice * usbFindDeviceByBus(unsigned int bus,
No space after * when returning a pointer type.
+ unsigned int devno);
+
+usbDeviceList * usbFindDeviceByVendor(unsigned int vendor,
Again
+ unsigned int product);
+
+usbDevice * usbFindDevice(unsigned int vendor,
Again
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)