2:27 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1171582
When we edit a negative controller address number to a device, some
of them will auto generate a controller with invalid index number. This
will make guest disappear after restart libvirtd. Instead of allow
negative number for controller index, I think we should forbid negative
number in these place (we did this before, but after f18c02ec,
virStrToLong_ui changed to allow negative number). So changed to use
virStrToLong_uip in these place.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/conf/device_conf.c | 4 ++--
src/conf/domain_conf.c | 32 ++++++++++++++++----------------
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index b3b04e1..a7ec8a6 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -71,14 +71,14 @@ virDevicePCIAddressParseXML(xmlNodePtr node,
}
if (bus &&
- virStrToLong_ui(bus, NULL, 0, &addr->bus) < 0) {
+ virStrToLong_uip(bus, NULL, 0, &addr->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'bus'
attribute"));
goto cleanup;
}
if (slot &&
- virStrToLong_ui(slot, NULL, 0, &addr->slot) < 0) {
+ virStrToLong_uip(slot, NULL, 0, &addr->slot) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'slot'
attribute"));
goto cleanup;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d81c37..961ec67 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3453,28 +3453,28 @@ virDomainDeviceDriveAddressParseXML(xmlNodePtr node,
unit = virXMLPropString(node, "unit");
if (controller &&
- virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
+ virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'controller'
attribute"));
goto cleanup;
}
if (bus &&
- virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
+ virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'bus'
attribute"));
goto cleanup;
}
if (target &&
- virStrToLong_ui(target, NULL, 10, &addr->target) < 0) {
+ virStrToLong_uip(target, NULL, 10, &addr->target) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'target'
attribute"));
goto cleanup;
}
if (unit &&
- virStrToLong_ui(unit, NULL, 10, &addr->unit) < 0) {
+ virStrToLong_uip(unit, NULL, 10, &addr->unit) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'unit'
attribute"));
goto cleanup;
@@ -3507,21 +3507,21 @@ virDomainDeviceVirtioSerialAddressParseXML(
port = virXMLPropString(node, "port");
if (controller &&
- virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
+ virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'controller'
attribute"));
goto cleanup;
}
if (bus &&
- virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
+ virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'bus'
attribute"));
goto cleanup;
}
if (port &&
- virStrToLong_ui(port, NULL, 10, &addr->port) < 0) {
+ virStrToLong_uip(port, NULL, 10, &addr->port) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'port'
attribute"));
goto cleanup;
@@ -3553,19 +3553,19 @@ virDomainDeviceCCWAddressParseXML(xmlNodePtr node,
if (cssid && ssid && devno) {
if (cssid &&
- virStrToLong_ui(cssid, NULL, 0, &addr->cssid) < 0) {
+ virStrToLong_uip(cssid, NULL, 0, &addr->cssid) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'cssid'
attribute"));
goto cleanup;
}
if (ssid &&
- virStrToLong_ui(ssid, NULL, 0, &addr->ssid) < 0) {
+ virStrToLong_uip(ssid, NULL, 0, &addr->ssid) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'ssid'
attribute"));
goto cleanup;
}
if (devno &&
- virStrToLong_ui(devno, NULL, 0, &addr->devno) < 0) {
+ virStrToLong_uip(devno, NULL, 0, &addr->devno) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'devno'
attribute"));
goto cleanup;
@@ -3607,14 +3607,14 @@ virDomainDeviceCcidAddressParseXML(xmlNodePtr node,
slot = virXMLPropString(node, "slot");
if (controller &&
- virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
+ virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'controller'
attribute"));
goto cleanup;
}
if (slot &&
- virStrToLong_ui(slot, NULL, 10, &addr->slot) < 0) {
+ virStrToLong_uip(slot, NULL, 10, &addr->slot) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'slot'
attribute"));
goto cleanup;
@@ -3642,7 +3642,7 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
bus = virXMLPropString(node, "bus");
if (port &&
- ((virStrToLong_ui(port, &tmp, 10, &p) < 0 || (*tmp != '\0'
&& *tmp != '.')) ||
+ ((virStrToLong_uip(port, &tmp, 10, &p) < 0 || (*tmp != '\0'
&& *tmp != '.')) ||
(*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0' && *tmp != '.'))) ||
(*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0' && *tmp != '.'))) ||
(*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0'))))) {
@@ -3655,7 +3655,7 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
port = NULL;
if (bus &&
- virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
+ virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'bus'
attribute"));
goto cleanup;
@@ -3777,14 +3777,14 @@ virDomainDeviceISAAddressParseXML(xmlNodePtr node,
irq = virXMLPropString(node, "irq");
if (iobase &&
- virStrToLong_ui(iobase, NULL, 16, &addr->iobase) < 0) {
+ virStrToLong_uip(iobase, NULL, 16, &addr->iobase) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Cannot parse <address> 'iobase'
attribute"));
goto cleanup;
}
if (irq &&
- virStrToLong_ui(irq, NULL, 16, &addr->irq) < 0) {
+ virStrToLong_uip(irq, NULL, 16, &addr->irq) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Cannot parse <address> 'irq'
attribute"));
goto cleanup;
--
1.8.3.1
4:38 a.m.
On 08.12.2014 09:27, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1171582
When we edit a negative controller address number to a device, some
of them will auto generate a controller with invalid index number. This
will make guest disappear after restart libvirtd. Instead of allow
negative number for controller index, I think we should forbid negative
number in these place (we did this before, but after f18c02ec,
virStrToLong_ui changed to allow negative number). So changed to use
virStrToLong_uip in these place.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/conf/device_conf.c | 4 ++--
src/conf/domain_conf.c | 32 ++++++++++++++++----------------
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index b3b04e1..a7ec8a6 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -71,14 +71,14 @@ virDevicePCIAddressParseXML(xmlNodePtr node,
}
if (bus &&
- virStrToLong_ui(bus, NULL, 0, &addr->bus) < 0) {
+ virStrToLong_uip(bus, NULL, 0, &addr->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'bus'
attribute"));
goto cleanup;
}
if (slot &&
- virStrToLong_ui(slot, NULL, 0, &addr->slot) < 0) {
+ virStrToLong_uip(slot, NULL, 0, &addr->slot) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'slot'
attribute"));
goto cleanup;
There are other attributes that you missed: domain and function. Even
though they are tested in virDevicePCIAddressIsValid() it is not
sufficient. For instance, for function you can have a number from 0 to 7
(including). Since function is an unsigned int, entering the correct
negative value that is equal to unsigned value from the range would
accept the value. But it shouldn't.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d81c37..961ec67 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3453,28 +3453,28 @@ virDomainDeviceDriveAddressParseXML(xmlNodePtr node,
unit = virXMLPropString(node, "unit");
if (controller &&
- virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
+ virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'controller'
attribute"));
goto cleanup;
}
if (bus &&
- virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
+ virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'bus'
attribute"));
goto cleanup;
}
if (target &&
- virStrToLong_ui(target, NULL, 10, &addr->target) < 0) {
+ virStrToLong_uip(target, NULL, 10, &addr->target) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'target'
attribute"));
goto cleanup;
}
if (unit &&
- virStrToLong_ui(unit, NULL, 10, &addr->unit) < 0) {
+ virStrToLong_uip(unit, NULL, 10, &addr->unit) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'unit'
attribute"));
goto cleanup;
@@ -3507,21 +3507,21 @@ virDomainDeviceVirtioSerialAddressParseXML(
port = virXMLPropString(node, "port");
if (controller &&
- virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
+ virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'controller'
attribute"));
goto cleanup;
}
if (bus &&
- virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
+ virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'bus'
attribute"));
goto cleanup;
}
if (port &&
- virStrToLong_ui(port, NULL, 10, &addr->port) < 0) {
+ virStrToLong_uip(port, NULL, 10, &addr->port) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'port'
attribute"));
goto cleanup;
@@ -3553,19 +3553,19 @@ virDomainDeviceCCWAddressParseXML(xmlNodePtr node,
if (cssid && ssid && devno) {
if (cssid &&
- virStrToLong_ui(cssid, NULL, 0, &addr->cssid) < 0) {
+ virStrToLong_uip(cssid, NULL, 0, &addr->cssid) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'cssid'
attribute"));
goto cleanup;
}
if (ssid &&
- virStrToLong_ui(ssid, NULL, 0, &addr->ssid) < 0) {
+ virStrToLong_uip(ssid, NULL, 0, &addr->ssid) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'ssid'
attribute"));
goto cleanup;
}
if (devno &&
- virStrToLong_ui(devno, NULL, 0, &addr->devno) < 0) {
+ virStrToLong_uip(devno, NULL, 0, &addr->devno) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'devno'
attribute"));
goto cleanup;
@@ -3607,14 +3607,14 @@ virDomainDeviceCcidAddressParseXML(xmlNodePtr node,
slot = virXMLPropString(node, "slot");
if (controller &&
- virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
+ virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'controller'
attribute"));
goto cleanup;
}
if (slot &&
- virStrToLong_ui(slot, NULL, 10, &addr->slot) < 0) {
+ virStrToLong_uip(slot, NULL, 10, &addr->slot) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'slot'
attribute"));
goto cleanup;
@@ -3642,7 +3642,7 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
bus = virXMLPropString(node, "bus");
if (port &&
- ((virStrToLong_ui(port, &tmp, 10, &p) < 0 || (*tmp != '\0'
&& *tmp != '.')) ||
+ ((virStrToLong_uip(port, &tmp, 10, &p) < 0 || (*tmp != '\0'
&& *tmp != '.')) ||
(*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10,
&p) < 0 || (*tmp != '\0' && *tmp != '.'))) ||
(*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10,
&p) < 0 || (*tmp != '\0' && *tmp != '.'))) ||
(*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10,
&p) < 0 || (*tmp != '\0'))))) {
@@ -3655,7 +3655,7 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
port = NULL;
if (bus &&
- virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
+ virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'bus'
attribute"));
goto cleanup;
@@ -3777,14 +3777,14 @@ virDomainDeviceISAAddressParseXML(xmlNodePtr node,
irq = virXMLPropString(node, "irq");
if (iobase &&
- virStrToLong_ui(iobase, NULL, 16, &addr->iobase) < 0) {
+ virStrToLong_uip(iobase, NULL, 16, &addr->iobase) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Cannot parse <address> 'iobase'
attribute"));
goto cleanup;
}
if (irq &&
- virStrToLong_ui(irq, NULL, 16, &addr->irq) < 0) {
+ virStrToLong_uip(irq, NULL, 16, &addr->irq) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Cannot parse <address> 'irq'
attribute"));
goto cleanup;
So while this patch is incomplete, I'll ACK and push it. I mean, there
are still plenty of places that accept a negative number but shouldn't.
But this makes situation better and that counts.
Fixed the missing attributes, ACked and pushed.
Side note, while I'd love to see a regression test case, I don't think
it's possible. We would need a separate XML for each attribute fixed so
the test suite would explode with plenty of similar XML files. Therefore
I think we are just okay without a test case.
Michal
6:57 a.m.
On 12/09/2014 06:38 PM, Michal Privoznik wrote:
On 08.12.2014 09:27, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1171582
>
> When we edit a negative controller address number to a device, some
> of them will auto generate a controller with invalid index number. This
> will make guest disappear after restart libvirtd. Instead of allow
> negative number for controller index, I think we should forbid negative
> number in these place (we did this before, but after f18c02ec,
> virStrToLong_ui changed to allow negative number). So changed to use
> virStrToLong_uip in these place.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/conf/device_conf.c | 4 ++--
> src/conf/domain_conf.c | 32 ++++++++++++++++----------------
> 2 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> index b3b04e1..a7ec8a6 100644
> --- a/src/conf/device_conf.c
> +++ b/src/conf/device_conf.c
> @@ -71,14 +71,14 @@ virDevicePCIAddressParseXML(xmlNodePtr node,
> }
>
> if (bus &&
> - virStrToLong_ui(bus, NULL, 0, &addr->bus) < 0) {
> + virStrToLong_uip(bus, NULL, 0, &addr->bus) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'bus'
attribute"));
> goto cleanup;
> }
>
> if (slot &&
> - virStrToLong_ui(slot, NULL, 0, &addr->slot) < 0) {
> + virStrToLong_uip(slot, NULL, 0, &addr->slot) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'slot'
attribute"));
> goto cleanup;
There are other attributes that you missed: domain and function. Even
though they are tested in virDevicePCIAddressIsValid() it is not
sufficient. For instance, for function you can have a number from 0 to
7 (including). Since function is an unsigned int, entering the correct
negative value that is equal to unsigned value from the range would
accept the value. But it shouldn't.
Thanks a lot for pointing out, I forgot them when i wrote this patch.
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2d81c37..961ec67 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3453,28 +3453,28 @@
> virDomainDeviceDriveAddressParseXML(xmlNodePtr node,
> unit = virXMLPropString(node, "unit");
>
> if (controller &&
> - virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
> + virStrToLong_uip(controller, NULL, 10, &addr->controller) <
> 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'controller'
> attribute"));
> goto cleanup;
> }
>
> if (bus &&
> - virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
> + virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'bus'
attribute"));
> goto cleanup;
> }
>
> if (target &&
> - virStrToLong_ui(target, NULL, 10, &addr->target) < 0) {
> + virStrToLong_uip(target, NULL, 10, &addr->target) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'target'
> attribute"));
> goto cleanup;
> }
>
> if (unit &&
> - virStrToLong_ui(unit, NULL, 10, &addr->unit) < 0) {
> + virStrToLong_uip(unit, NULL, 10, &addr->unit) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'unit'
attribute"));
> goto cleanup;
> @@ -3507,21 +3507,21 @@ virDomainDeviceVirtioSerialAddressParseXML(
> port = virXMLPropString(node, "port");
>
> if (controller &&
> - virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
> + virStrToLong_uip(controller, NULL, 10, &addr->controller) <
> 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'controller'
> attribute"));
> goto cleanup;
> }
>
> if (bus &&
> - virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
> + virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'bus'
attribute"));
> goto cleanup;
> }
>
> if (port &&
> - virStrToLong_ui(port, NULL, 10, &addr->port) < 0) {
> + virStrToLong_uip(port, NULL, 10, &addr->port) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'port'
attribute"));
> goto cleanup;
> @@ -3553,19 +3553,19 @@ virDomainDeviceCCWAddressParseXML(xmlNodePtr
> node,
>
> if (cssid && ssid && devno) {
> if (cssid &&
> - virStrToLong_ui(cssid, NULL, 0, &addr->cssid) < 0) {
> + virStrToLong_uip(cssid, NULL, 0, &addr->cssid) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'cssid'
> attribute"));
> goto cleanup;
> }
> if (ssid &&
> - virStrToLong_ui(ssid, NULL, 0, &addr->ssid) < 0) {
> + virStrToLong_uip(ssid, NULL, 0, &addr->ssid) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'ssid'
> attribute"));
> goto cleanup;
> }
> if (devno &&
> - virStrToLong_ui(devno, NULL, 0, &addr->devno) < 0) {
> + virStrToLong_uip(devno, NULL, 0, &addr->devno) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'devno'
> attribute"));
> goto cleanup;
> @@ -3607,14 +3607,14 @@ virDomainDeviceCcidAddressParseXML(xmlNodePtr
> node,
> slot = virXMLPropString(node, "slot");
>
> if (controller &&
> - virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
> + virStrToLong_uip(controller, NULL, 10, &addr->controller) <
> 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'controller'
> attribute"));
> goto cleanup;
> }
>
> if (slot &&
> - virStrToLong_ui(slot, NULL, 10, &addr->slot) < 0) {
> + virStrToLong_uip(slot, NULL, 10, &addr->slot) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'slot'
attribute"));
> goto cleanup;
> @@ -3642,7 +3642,7 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
> bus = virXMLPropString(node, "bus");
>
> if (port &&
> - ((virStrToLong_ui(port, &tmp, 10, &p) < 0 || (*tmp !=
'\0'
> && *tmp != '.')) ||
> + ((virStrToLong_uip(port, &tmp, 10, &p) < 0 || (*tmp !=
'\0'
> && *tmp != '.')) ||
> (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10,
&p) <
> 0 || (*tmp != '\0' && *tmp != '.'))) ||
> (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10,
&p) <
> 0 || (*tmp != '\0' && *tmp != '.'))) ||
> (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10,
&p) <
> 0 || (*tmp != '\0'))))) {
> @@ -3655,7 +3655,7 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
> port = NULL;
>
> if (bus &&
> - virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
> + virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot parse <address> 'bus'
attribute"));
> goto cleanup;
> @@ -3777,14 +3777,14 @@ virDomainDeviceISAAddressParseXML(xmlNodePtr
> node,
> irq = virXMLPropString(node, "irq");
>
> if (iobase &&
> - virStrToLong_ui(iobase, NULL, 16, &addr->iobase) < 0) {
> + virStrToLong_uip(iobase, NULL, 16, &addr->iobase) < 0) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("Cannot parse <address> 'iobase'
> attribute"));
> goto cleanup;
> }
>
> if (irq &&
> - virStrToLong_ui(irq, NULL, 16, &addr->irq) < 0) {
> + virStrToLong_uip(irq, NULL, 16, &addr->irq) < 0) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("Cannot parse <address> 'irq'
attribute"));
> goto cleanup;
>
So while this patch is incomplete, I'll ACK and push it. I mean, there
are still plenty of places that accept a negative number but
shouldn't. But this makes situation better and that counts.
Fixed the missing attributes, ACked and pushed.
Thanks a lot for your help, because
i am afraid i can't find all the
place should fix by myself ;)
Side note, while I'd love to see a regression test case, I don't think
it's possible. We would need a separate XML for each attribute fixed
so the test suite would explode with plenty of similar XML files.
Therefore I think we are just okay without a test case.
Michal
Luyao
3638
days inactive
3639
days old
2 comments
2 participants
participants (2)
-
Luyao Huang -
Michal Privoznik