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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list