8:46 a.m.
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(a)laine.org>
Signed-off-by: Ryota Ozaki <ozaki.ryota(a)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
8:46 a.m.
New subject: [libvirt] [PATCH 2/3] vbox: handle errors of virDomainHostdevDefAlloc correctly
The original code ignored errors of virDomainHostdevDefAlloc,
however, we should properly do error return from the function
if it occurs.
The fix pulls out virDomainHostdevDefAlloc from the loop and
executes it all together before the loop. So we can easily
return on errors without the notion of other memory allocations
in the loop.
The deallocation code is separated from the allocation code
because it will be used by a further patch for fixing other error
handlings.
Reported-by: Laine Stump <laine(a)laine.org>
Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com>
---
src/vbox/vbox_tmpl.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index cc5f275..9336514 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2269,6 +2269,12 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
if (VIR_ALLOC_N(def->hostdevs, def->nhostdevs) < 0)
goto release_filters;
+ for (i = 0; i < def->nhostdevs; i++) {
+ def->hostdevs[i] = virDomainHostdevDefAlloc();
+ if (!def->hostdevs[i])
+ goto release_hostdevs;
+ }
+
for (i = 0; i < deviceFilters.count; i++) {
PRBool active = PR_FALSE;
IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
@@ -2284,10 +2290,6 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
if (!active)
continue;
- def->hostdevs[USBFilterCount] = virDomainHostdevDefAlloc();
- if (!def->hostdevs[USBFilterCount])
- continue;
-
def->hostdevs[USBFilterCount]->mode =
VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
def->hostdevs[USBFilterCount]->source.subsys.type =
@@ -2322,6 +2324,15 @@ release_controller:
#else
VBOX_RELEASE(USBDeviceFilters);
#endif
+
+ return;
+
+release_hostdevs:
+ for (i = 0; i < def->nhostdevs; i++)
+ virDomainHostdevDefFree(def->hostdevs[i]);
+ VIR_FREE(def->hostdevs);
+
+ goto release_filters;
}
static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
--
1.8.4
9:02 p.m.
New subject: [libvirt] [PATCH 2/3] vbox: handle errors of virDomainHostdevDefAlloc correctly
Okay, that sounds fine, applied and pushed too,
thank !
Daniel
On Sun, Dec 01, 2013 at 11:46:06PM +0900, Ryota Ozaki wrote:
The original code ignored errors of virDomainHostdevDefAlloc,
however, we should properly do error return from the function
if it occurs.
The fix pulls out virDomainHostdevDefAlloc from the loop and
executes it all together before the loop. So we can easily
return on errors without the notion of other memory allocations
in the loop.
The deallocation code is separated from the allocation code
because it will be used by a further patch for fixing other error
handlings.
Reported-by: Laine Stump <laine(a)laine.org>
Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com>
---
src/vbox/vbox_tmpl.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index cc5f275..9336514 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2269,6 +2269,12 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
if (VIR_ALLOC_N(def->hostdevs, def->nhostdevs) < 0)
goto release_filters;
+ for (i = 0; i < def->nhostdevs; i++) {
+ def->hostdevs[i] = virDomainHostdevDefAlloc();
+ if (!def->hostdevs[i])
+ goto release_hostdevs;
+ }
+
for (i = 0; i < deviceFilters.count; i++) {
PRBool active = PR_FALSE;
IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
@@ -2284,10 +2290,6 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
if (!active)
continue;
- def->hostdevs[USBFilterCount] = virDomainHostdevDefAlloc();
- if (!def->hostdevs[USBFilterCount])
- continue;
-
def->hostdevs[USBFilterCount]->mode =
VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
def->hostdevs[USBFilterCount]->source.subsys.type =
@@ -2322,6 +2324,15 @@ release_controller:
#else
VBOX_RELEASE(USBDeviceFilters);
#endif
+
+ return;
+
+release_hostdevs:
+ for (i = 0; i < def->nhostdevs; i++)
+ virDomainHostdevDefFree(def->hostdevs[i]);
+ VIR_FREE(def->hostdevs);
+
+ goto release_filters;
}
static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
--
1.8.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
8:46 a.m.
New subject: [libvirt] [PATCH 3/3] vbox: handle several errors correctly in vboxHostDeviceGetXMLDesc
In the original code, errors of Get{Vendor,Product}Id,
VBOX_UTF16_TO_UTF8, strtol were ignored.
In order to handle the errors, the patch introduces
vboxGetVendorProductIds utility function because adding error
codes in the loop is difficult.
Reported-by: Laine Stump <laine(a)laine.org>
Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com>
---
src/vbox/vbox_tmpl.c | 74 ++++++++++++++++++++++++++++++++++++----------------
1 file changed, 51 insertions(+), 23 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 9336514..27c21bd 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2208,6 +2208,51 @@ vboxDomainGetMaxVcpus(virDomainPtr dom)
VIR_DOMAIN_VCPU_MAXIMUM));
}
+static int vboxGetVendorProductIds(vboxGlobalData *data,
+ IUSBDeviceFilter *deviceFilter,
+ unsigned *vendorId,
+ unsigned *productId)
+{
+ PRUnichar *idUtf16 = NULL;
+ char *idUtf8 = NULL;
+ char *endptr = NULL;
+ unsigned vId = 0;
+ unsigned pId = 0;
+ int result = -1;
+
+ if (deviceFilter->vtbl->GetVendorId(deviceFilter, &idUtf16) != 0)
+ goto out;
+
+ if (VBOX_UTF16_TO_UTF8(idUtf16, &idUtf8) < 0)
+ goto release_utf16;
+
+ if (virStrToLong_ui(idUtf8, &endptr, 16, &vId) < 0)
+ goto release_utf8;
+
+ VBOX_UTF8_FREE(idUtf8);
+ VBOX_UTF16_FREE(idUtf16);
+
+ if (deviceFilter->vtbl->GetProductId(deviceFilter, &idUtf16) != 0)
+ goto out;
+
+ if (VBOX_UTF16_TO_UTF8(idUtf16, &idUtf8) < 0)
+ goto release_utf16;
+
+ if (virStrToLong_ui(idUtf8, &endptr, 16, &pId) < 0)
+ goto release_utf8;
+
+ *vendorId = vId;
+ *productId = pId;
+ result = 0;
+
+release_utf8:
+ VBOX_UTF8_FREE(idUtf8);
+release_utf16:
+ VBOX_UTF16_FREE(idUtf16);
+out:
+ return result;
+}
+
static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr def, IMachine
*machine)
{
#if VBOX_API_VERSION < 4003
@@ -2278,13 +2323,7 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
for (i = 0; i < deviceFilters.count; i++) {
PRBool active = PR_FALSE;
IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
- PRUnichar *vendorIdUtf16 = NULL;
- char *vendorIdUtf8 = NULL;
- unsigned vendorId = 0;
- PRUnichar *productIdUtf16 = NULL;
- char *productIdUtf8 = NULL;
- unsigned productId = 0;
- char *endptr = NULL;
+ int res = 0;
deviceFilter->vtbl->GetActive(deviceFilter, &active);
if (!active)
@@ -2295,23 +2334,12 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
def->hostdevs[USBFilterCount]->source.subsys.type =
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB;
- deviceFilter->vtbl->GetVendorId(deviceFilter, &vendorIdUtf16);
- deviceFilter->vtbl->GetProductId(deviceFilter, &productIdUtf16);
+ res = vboxGetVendorProductIds(data, deviceFilter,
+
&def->hostdevs[USBFilterCount]->source.subsys.u.usb.vendor,
+
&def->hostdevs[USBFilterCount]->source.subsys.u.usb.product);
- VBOX_UTF16_TO_UTF8(vendorIdUtf16, &vendorIdUtf8);
- VBOX_UTF16_TO_UTF8(productIdUtf16, &productIdUtf8);
-
- vendorId = strtol(vendorIdUtf8, &endptr, 16);
- productId = strtol(productIdUtf8, &endptr, 16);
-
- def->hostdevs[USBFilterCount]->source.subsys.u.usb.vendor = vendorId;
- def->hostdevs[USBFilterCount]->source.subsys.u.usb.product = productId;
-
- VBOX_UTF16_FREE(vendorIdUtf16);
- VBOX_UTF8_FREE(vendorIdUtf8);
-
- VBOX_UTF16_FREE(productIdUtf16);
- VBOX_UTF8_FREE(productIdUtf8);
+ if (res < 0)
+ goto release_hostdevs;
USBFilterCount++;
}
--
1.8.4
9:03 p.m.
New subject: [libvirt] [PATCH 3/3] vbox: handle several errors correctly in vboxHostDeviceGetXMLDesc
This patch also looks fine, but is more complex, I would rather
postpone it after the 1.2.0 release,
Daniel
On Sun, Dec 01, 2013 at 11:46:07PM +0900, Ryota Ozaki wrote:
In the original code, errors of Get{Vendor,Product}Id,
VBOX_UTF16_TO_UTF8, strtol were ignored.
In order to handle the errors, the patch introduces
vboxGetVendorProductIds utility function because adding error
codes in the loop is difficult.
Reported-by: Laine Stump <laine(a)laine.org>
Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com>
---
src/vbox/vbox_tmpl.c | 74 ++++++++++++++++++++++++++++++++++++----------------
1 file changed, 51 insertions(+), 23 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 9336514..27c21bd 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2208,6 +2208,51 @@ vboxDomainGetMaxVcpus(virDomainPtr dom)
VIR_DOMAIN_VCPU_MAXIMUM));
}
+static int vboxGetVendorProductIds(vboxGlobalData *data,
+ IUSBDeviceFilter *deviceFilter,
+ unsigned *vendorId,
+ unsigned *productId)
+{
+ PRUnichar *idUtf16 = NULL;
+ char *idUtf8 = NULL;
+ char *endptr = NULL;
+ unsigned vId = 0;
+ unsigned pId = 0;
+ int result = -1;
+
+ if (deviceFilter->vtbl->GetVendorId(deviceFilter, &idUtf16) != 0)
+ goto out;
+
+ if (VBOX_UTF16_TO_UTF8(idUtf16, &idUtf8) < 0)
+ goto release_utf16;
+
+ if (virStrToLong_ui(idUtf8, &endptr, 16, &vId) < 0)
+ goto release_utf8;
+
+ VBOX_UTF8_FREE(idUtf8);
+ VBOX_UTF16_FREE(idUtf16);
+
+ if (deviceFilter->vtbl->GetProductId(deviceFilter, &idUtf16) != 0)
+ goto out;
+
+ if (VBOX_UTF16_TO_UTF8(idUtf16, &idUtf8) < 0)
+ goto release_utf16;
+
+ if (virStrToLong_ui(idUtf8, &endptr, 16, &pId) < 0)
+ goto release_utf8;
+
+ *vendorId = vId;
+ *productId = pId;
+ result = 0;
+
+release_utf8:
+ VBOX_UTF8_FREE(idUtf8);
+release_utf16:
+ VBOX_UTF16_FREE(idUtf16);
+out:
+ return result;
+}
+
static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr def, IMachine
*machine)
{
#if VBOX_API_VERSION < 4003
@@ -2278,13 +2323,7 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
for (i = 0; i < deviceFilters.count; i++) {
PRBool active = PR_FALSE;
IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
- PRUnichar *vendorIdUtf16 = NULL;
- char *vendorIdUtf8 = NULL;
- unsigned vendorId = 0;
- PRUnichar *productIdUtf16 = NULL;
- char *productIdUtf8 = NULL;
- unsigned productId = 0;
- char *endptr = NULL;
+ int res = 0;
deviceFilter->vtbl->GetActive(deviceFilter, &active);
if (!active)
@@ -2295,23 +2334,12 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
def->hostdevs[USBFilterCount]->source.subsys.type =
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB;
- deviceFilter->vtbl->GetVendorId(deviceFilter, &vendorIdUtf16);
- deviceFilter->vtbl->GetProductId(deviceFilter, &productIdUtf16);
+ res = vboxGetVendorProductIds(data, deviceFilter,
+
&def->hostdevs[USBFilterCount]->source.subsys.u.usb.vendor,
+
&def->hostdevs[USBFilterCount]->source.subsys.u.usb.product);
- VBOX_UTF16_TO_UTF8(vendorIdUtf16, &vendorIdUtf8);
- VBOX_UTF16_TO_UTF8(productIdUtf16, &productIdUtf8);
-
- vendorId = strtol(vendorIdUtf8, &endptr, 16);
- productId = strtol(productIdUtf8, &endptr, 16);
-
- def->hostdevs[USBFilterCount]->source.subsys.u.usb.vendor = vendorId;
- def->hostdevs[USBFilterCount]->source.subsys.u.usb.product = productId;
-
- VBOX_UTF16_FREE(vendorIdUtf16);
- VBOX_UTF8_FREE(vendorIdUtf8);
-
- VBOX_UTF16_FREE(productIdUtf16);
- VBOX_UTF8_FREE(productIdUtf8);
+ if (res < 0)
+ goto release_hostdevs;
USBFilterCount++;
}
--
1.8.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
6:49 a.m.
New subject: [libvirt] [PATCH 3/3] vbox: handle several errors correctly in vboxHostDeviceGetXMLDesc
On Mon, Dec 2, 2013 at 12:03 PM, Daniel Veillard <veillard(a)redhat.com> wrote:
This patch also looks fine, but is more complex, I would rather
postpone it after the 1.2.0 release,
Okay, not a problem :)
I will check the patch again and resend it later.
Thanks,
ozaki-r
Daniel
On Sun, Dec 01, 2013 at 11:46:07PM +0900, Ryota Ozaki wrote:
> In the original code, errors of Get{Vendor,Product}Id,
> VBOX_UTF16_TO_UTF8, strtol were ignored.
>
> In order to handle the errors, the patch introduces
> vboxGetVendorProductIds utility function because adding error
> codes in the loop is difficult.
>
> Reported-by: Laine Stump <laine(a)laine.org>
> Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com>
> ---
> src/vbox/vbox_tmpl.c | 74 ++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 51 insertions(+), 23 deletions(-)
>
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 9336514..27c21bd 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -2208,6 +2208,51 @@ vboxDomainGetMaxVcpus(virDomainPtr dom)
> VIR_DOMAIN_VCPU_MAXIMUM));
> }
>
> +static int vboxGetVendorProductIds(vboxGlobalData *data,
> + IUSBDeviceFilter *deviceFilter,
> + unsigned *vendorId,
> + unsigned *productId)
> +{
> + PRUnichar *idUtf16 = NULL;
> + char *idUtf8 = NULL;
> + char *endptr = NULL;
> + unsigned vId = 0;
> + unsigned pId = 0;
> + int result = -1;
> +
> + if (deviceFilter->vtbl->GetVendorId(deviceFilter, &idUtf16) != 0)
> + goto out;
> +
> + if (VBOX_UTF16_TO_UTF8(idUtf16, &idUtf8) < 0)
> + goto release_utf16;
> +
> + if (virStrToLong_ui(idUtf8, &endptr, 16, &vId) < 0)
> + goto release_utf8;
> +
> + VBOX_UTF8_FREE(idUtf8);
> + VBOX_UTF16_FREE(idUtf16);
> +
> + if (deviceFilter->vtbl->GetProductId(deviceFilter, &idUtf16) != 0)
> + goto out;
> +
> + if (VBOX_UTF16_TO_UTF8(idUtf16, &idUtf8) < 0)
> + goto release_utf16;
> +
> + if (virStrToLong_ui(idUtf8, &endptr, 16, &pId) < 0)
> + goto release_utf8;
> +
> + *vendorId = vId;
> + *productId = pId;
> + result = 0;
> +
> +release_utf8:
> + VBOX_UTF8_FREE(idUtf8);
> +release_utf16:
> + VBOX_UTF16_FREE(idUtf16);
> +out:
> + return result;
> +}
> +
> static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr def,
IMachine *machine)
> {
> #if VBOX_API_VERSION < 4003
> @@ -2278,13 +2323,7 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
> for (i = 0; i < deviceFilters.count; i++) {
> PRBool active = PR_FALSE;
> IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
> - PRUnichar *vendorIdUtf16 = NULL;
> - char *vendorIdUtf8 = NULL;
> - unsigned vendorId = 0;
> - PRUnichar *productIdUtf16 = NULL;
> - char *productIdUtf8 = NULL;
> - unsigned productId = 0;
> - char *endptr = NULL;
> + int res = 0;
>
> deviceFilter->vtbl->GetActive(deviceFilter, &active);
> if (!active)
> @@ -2295,23 +2334,12 @@ static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data,
virDomainDefPtr def,
> def->hostdevs[USBFilterCount]->source.subsys.type =
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB;
>
> - deviceFilter->vtbl->GetVendorId(deviceFilter, &vendorIdUtf16);
> - deviceFilter->vtbl->GetProductId(deviceFilter, &productIdUtf16);
> + res = vboxGetVendorProductIds(data, deviceFilter,
> +
&def->hostdevs[USBFilterCount]->source.subsys.u.usb.vendor,
> +
&def->hostdevs[USBFilterCount]->source.subsys.u.usb.product);
>
> - VBOX_UTF16_TO_UTF8(vendorIdUtf16, &vendorIdUtf8);
> - VBOX_UTF16_TO_UTF8(productIdUtf16, &productIdUtf8);
> -
> - vendorId = strtol(vendorIdUtf8, &endptr, 16);
> - productId = strtol(productIdUtf8, &endptr, 16);
> -
> - def->hostdevs[USBFilterCount]->source.subsys.u.usb.vendor =
vendorId;
> - def->hostdevs[USBFilterCount]->source.subsys.u.usb.product =
productId;
> -
> - VBOX_UTF16_FREE(vendorIdUtf16);
> - VBOX_UTF8_FREE(vendorIdUtf8);
> -
> - VBOX_UTF16_FREE(productIdUtf16);
> - VBOX_UTF8_FREE(productIdUtf8);
> + if (res < 0)
> + goto release_hostdevs;
>
> USBFilterCount++;
> }
> --
> 1.8.4
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
8:58 p.m.
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(a)laine.org>
Signed-off-by: Ryota Ozaki <ozaki.ryota(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
4008
days inactive
4009
days old
6 comments
2 participants
participants (2)
-
Daniel Veillard -
Ryota Ozaki