[libvirt] [PATCH] conf: forbid negative number in address(like controller, bus, slot...)
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@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
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@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
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@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. Thanks a lot for your help, because i am afraid i can't find all the
Thanks a lot for pointing out, I forgot them when i wrote this patch. 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
participants (2)
-
Luyao Huang -
Michal Privoznik