[libvirt] [PATCH 1/3] vbox: fix incorrect loop condition in vboxHostDeviceGetXMLDesc

[Daniel Veillard]

  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 at laine.org>
> Signed-off-by: Ryota Ozaki <ozaki.ryota at gmail.com>
> ---
>  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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-- 
Daniel Veillard      | Open Source and Standards, Red Hat
veillard at redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/