ACK looks the right thing to do indeed, thanks, Daniel On Sun, Dec 01, 2013 at 11:46:05PM +0900, Ryota Ozaki wrote: > The fixed loop used logical OR to combine two conditions, however, > it is apparently incorrect and logical AND is correct. > > We can fix it by replacing OR with AND, but this patch instead > fixes the problem by getting rid of the first conditional > statement: USBFilterCount < def->nhostdevs. It isn't needed > because USBFilterCount will never be greater than or equal to > def->nhostdevs. > > def->nhostdevs is calculated in the following code > above the loop in question like this: > > for (i = 0; i < deviceFilters.count; i++) { > PRBool active = PR_FALSE; > IUSBDeviceFilter *deviceFilter = deviceFilters.items[i]; > > deviceFilter->vtbl->GetActive(deviceFilter, &active); > if (active) { > def->nhostdevs++; > } > } > > And the loop is constructed as like this: > > for (i = 0; (USBFilterCount < def->nhostdevs) || (i < deviceFilters.count); i++) { > PRBool active = PR_FALSE; > (snip) > deviceFilter->vtbl->GetActive(deviceFilter, &active); > if (!active) > continue; > (snip) > USBFilterCount++; > } > > So def->nhostdevs is the number of active device filters and > USBFilterCount is counted up only when a device filter is active. > Thus, we can remove USBFilterCount < def->nhostdevs safely. > > Reported-by: Laine Stump <laine@xxxxxxxxx> > Signed-off-by: Ryota Ozaki <ozaki.ryota@xxxxxxxxx> > --- > src/vbox/vbox_tmpl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c > index 983a595..cc5f275 100644 > --- a/src/vbox/vbox_tmpl.c > +++ b/src/vbox/vbox_tmpl.c > @@ -2269,7 +2269,7 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr def, > if (VIR_ALLOC_N(def->hostdevs, def->nhostdevs) < 0) > goto release_filters; > > - for (i = 0; (USBFilterCount < def->nhostdevs) || (i < deviceFilters.count); i++) { > + for (i = 0; i < deviceFilters.count; i++) { > PRBool active = PR_FALSE; > IUSBDeviceFilter *deviceFilter = deviceFilters.items[i]; > PRUnichar *vendorIdUtf16 = NULL; > -- > 1.8.4 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- Daniel Veillard | Open Source and Standards, Red Hat veillard@xxxxxxxxxx | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list