[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virDomainBackupDiskDefParseXML: Use virXMLProp* virDomainBackupDefParse: Use virXMLProp* virZPCIDeviceAddressParseXML: Use virXMLProp* virPCIDeviceAddressParseXML: Use virXMLProp* virDomainDeviceCCWAddressParseXML: Use virXMLProp* virDomainDeviceDriveAddressParseXML: Use virXMLProp* virDomainDeviceVirtioSerialAddressParseXML: Use virXMLProp* virDomainDeviceCcidAddressParseXML: Use virXMLProp* virDomainDeviceUSBAddressParseXML: Use virXMLProp* virInterfaceLinkParseXML: Use virXMLProp* src/conf/backup_conf.c | 57 +++------- src/conf/device_conf.c | 241 ++++++++++++----------------------------- 2 files changed, 84 insertions(+), 214 deletions(-) -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/backup_conf.c | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 2de77a59c0..859609946f 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -106,10 +106,6 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node, g_autofree char *type = NULL; g_autofree char *format = NULL; g_autofree char *idx = NULL; - g_autofree char *backup = NULL; - g_autofree char *state = NULL; - g_autofree char *backupmode = NULL; - int tmp; xmlNodePtr srcNode; unsigned int storageSourceParseFlags = 0; bool internal = flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL; @@ -127,15 +123,9 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node, def->backup = VIR_TRISTATE_BOOL_YES; - if ((backup = virXMLPropString(node, "backup"))) { - if ((tmp = virTristateBoolTypeFromString(backup)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid disk 'backup' state '%s'"), backup); - return -1; - } - - def->backup = tmp; - } + if (virXMLPropTristateBool(node, "backup", VIR_XML_PROP_OPTIONAL, + &def->backup) < 0) + return -1; /* don't parse anything else if backup is disabled */ if (def->backup == VIR_TRISTATE_BOOL_NO) @@ -146,28 +136,18 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node, def->exportbitmap = virXMLPropString(node, "exportbitmap"); } - if ((backupmode = virXMLPropString(node, "backupmode"))) { - if ((tmp = virDomainBackupDiskBackupModeTypeFromString(backupmode)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid backupmode '%s' of disk '%s'"), - backupmode, def->name); - return -1; - } - - def->backupmode = tmp; - } + if (virXMLPropEnum(node, "backupmode", + virDomainBackupDiskBackupModeTypeFromString, + VIR_XML_PROP_OPTIONAL, &def->backupmode) < 0) + return -1; def->incremental = virXMLPropString(node, "incremental"); if (internal) { - if (!(state = virXMLPropString(node, "state")) || - (tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("disk '%s' backup state wrong or missing'"), def->name); + if (virXMLPropEnum(node, "state", + virDomainBackupDiskStateTypeFromString, + VIR_XML_PROP_REQUIRED, &def->state) < 0) return -1; - } - - def->state = tmp; } type = virXMLPropString(node, "type"); -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/backup_conf.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 859609946f..898725aac7 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -222,8 +222,6 @@ virDomainBackupDefParse(xmlXPathContextPtr ctxt, def->incremental = virXPathString("string(./incremental)", ctxt); if ((node = virXPathNode("./server", ctxt))) { - g_autofree char *tls = NULL; - if (def->type != VIR_DOMAIN_BACKUP_TYPE_PULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("use of <server> requires pull mode backup")); @@ -249,18 +247,9 @@ virDomainBackupDefParse(xmlXPathContextPtr ctxt, return NULL; } - if ((tls = virXMLPropString(node, "tls"))) { - int tmp; - - if ((tmp = virTristateBoolTypeFromString(tls)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown value '%s' of 'tls' attribute"),\ - tls); - return NULL; - } - - def->tls = tmp; - } + if (virXMLPropTristateBool(node, "tls", VIR_XML_PROP_OPTIONAL, + &def->tls) < 0) + return NULL; } if ((n = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0) -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 6a4b14cfda..8d3d012e4a 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -52,32 +52,21 @@ static int virZPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddress *addr) { - virZPCIDeviceAddress def = { .uid = { 0 }, .fid = { 0 } }; - g_autofree char *uid = NULL; - g_autofree char *fid = NULL; + int retUid, retFid; - uid = virXMLPropString(node, "uid"); - fid = virXMLPropString(node, "fid"); + if ((retUid = virXMLPropUInt(node, "uid", 0, VIR_XML_PROP_OPTIONAL, + &addr->zpci.uid.value)) < 0) + return -1; - if (uid) { - if (virStrToLong_uip(uid, NULL, 0, &def.uid.value) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'uid' attribute")); - return -1; - } - def.uid.isSet = true; - } + if (retUid > 0) + addr->zpci.uid.isSet = true; - if (fid) { - if (virStrToLong_uip(fid, NULL, 0, &def.fid.value) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'fid' attribute")); - return -1; - } - def.fid.isSet = true; - } + if ((retFid = virXMLPropUInt(node, "fid", 0, VIR_XML_PROP_OPTIONAL, + &addr->zpci.fid.value)) < 0) + return -1; - addr->zpci = def; + if (retFid > 0) + addr->zpci.fid.isSet = true; return 0; } -- 2.26.2
On Fri, Apr 16, 2021 at 14:20:53 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 6a4b14cfda..8d3d012e4a 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -52,32 +52,21 @@ static int virZPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddress *addr) { - virZPCIDeviceAddress def = { .uid = { 0 }, .fid = { 0 } }; - g_autofree char *uid = NULL; - g_autofree char *fid = NULL; + int retUid, retFid;
Don't declare 2 variables on a signle line.
- uid = virXMLPropString(node, "uid"); - fid = virXMLPropString(node, "fid"); + if ((retUid = virXMLPropUInt(node, "uid", 0, VIR_XML_PROP_OPTIONAL, + &addr->zpci.uid.value)) < 0)
Second line is misaligned.
+ return -1;
- if (uid) { - if (virStrToLong_uip(uid, NULL, 0, &def.uid.value) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'uid' attribute")); - return -1; - } - def.uid.isSet = true; - } + if (retUid > 0) + addr->zpci.uid.isSet = true;
- if (fid) { - if (virStrToLong_uip(fid, NULL, 0, &def.fid.value) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'fid' attribute")); - return -1; - } - def.fid.isSet = true; - } + if ((retFid = virXMLPropUInt(node, "fid", 0, VIR_XML_PROP_OPTIONAL, + &addr->zpci.fid.value)) < 0)
Same here.
+ return -1;
- addr->zpci = def; + if (retFid > 0) + addr->zpci.fid.isSet = true;
return 0; } -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 47 +++++++++++------------------------------- 1 file changed, 12 insertions(+), 35 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 8d3d012e4a..951b7a348e 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -199,52 +199,29 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, { xmlNodePtr cur; xmlNodePtr zpci = NULL; - g_autofree char *domain = virXMLPropString(node, "domain"); - g_autofree char *bus = virXMLPropString(node, "bus"); - g_autofree char *slot = virXMLPropString(node, "slot"); - g_autofree char *function = virXMLPropString(node, "function"); - g_autofree char *multi = virXMLPropString(node, "multifunction"); memset(addr, 0, sizeof(*addr)); - if (domain && - virStrToLong_uip(domain, NULL, 0, &addr->domain) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'domain' attribute")); + if (virXMLPropUInt(node, "domain", 0, VIR_XML_PROP_OPTIONAL, + &addr->domain) < 0) return -1; - } - if (bus && - virStrToLong_uip(bus, NULL, 0, &addr->bus) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'bus' attribute")); + if (virXMLPropUInt(node, "bus", 0, VIR_XML_PROP_OPTIONAL, + &addr->bus) < 0) return -1; - } - if (slot && - virStrToLong_uip(slot, NULL, 0, &addr->slot) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'slot' attribute")); + if (virXMLPropUInt(node, "slot", 0, VIR_XML_PROP_OPTIONAL, + &addr->slot) < 0) return -1; - } - if (function && - virStrToLong_uip(function, NULL, 0, &addr->function) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'function' attribute")); + if (virXMLPropUInt(node, "function", 0, VIR_XML_PROP_OPTIONAL, + &addr->function) < 0) + return -1; + + if (virXMLPropTristateSwitch(node, "multifunction", VIR_XML_PROP_OPTIONAL, + &addr->multi) < 0) return -1; - } - if (multi) { - int value; - if ((value = virTristateSwitchTypeFromString(multi)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown value '%s' for <address> 'multifunction' attribute"), - multi); - return -1; - } - addr->multi = value; - } if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true)) return -1; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 51 +++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 951b7a348e..621ff1b476 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -266,43 +266,34 @@ int virDomainDeviceCCWAddressParseXML(xmlNodePtr node, virDomainDeviceCCWAddress *addr) { - g_autofree char *cssid = virXMLPropString(node, "cssid"); - g_autofree char *ssid = virXMLPropString(node, "ssid"); - g_autofree char *devno = virXMLPropString(node, "devno"); + int cssid, ssid, devno; memset(addr, 0, sizeof(*addr)); + if ((cssid = virXMLPropUInt(node, "cssid", 0, VIR_XML_PROP_OPTIONAL, + &addr->cssid)) < 0) + return -1; + + if ((ssid = virXMLPropUInt(node, "ssid", 0, VIR_XML_PROP_OPTIONAL, + &addr->ssid)) < 0) + return -1; + + if ((devno = virXMLPropUInt(node, "devno", 0, VIR_XML_PROP_OPTIONAL, + &addr->devno)) < 0) + return -1; + + if (!virDomainDeviceCCWAddressIsValid(addr)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid specification for virtio ccw address: cssid='%u' ssid='%u' devno='%u'"), + addr->cssid, addr->ssid, addr->devno); + return -1; + } + if (cssid && ssid && devno) { - if (cssid && - virStrToLong_uip(cssid, NULL, 0, &addr->cssid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'cssid' attribute")); - return -1; - } - if (ssid && - virStrToLong_uip(ssid, NULL, 0, &addr->ssid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'ssid' attribute")); - return -1; - } - if (devno && - virStrToLong_uip(devno, NULL, 0, &addr->devno) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'devno' attribute")); - return -1; - } - if (!virDomainDeviceCCWAddressIsValid(addr)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid specification for virtio ccw" - " address: cssid='%s' ssid='%s' devno='%s'"), - cssid, ssid, devno); - return -1; - } addr->assigned = true; } else if (cssid || ssid || devno) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Invalid partial specification for virtio ccw" - " address")); + _("Invalid partial specification for virtio ccw address")); return -1; } -- 2.26.2
On Fri, Apr 16, 2021 at 14:20:55 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 51 +++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 951b7a348e..621ff1b476 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -266,43 +266,34 @@ int virDomainDeviceCCWAddressParseXML(xmlNodePtr node, virDomainDeviceCCWAddress *addr) { - g_autofree char *cssid = virXMLPropString(node, "cssid"); - g_autofree char *ssid = virXMLPropString(node, "ssid"); - g_autofree char *devno = virXMLPropString(node, "devno"); + int cssid, ssid, devno;
No multiple variable declarations on single line.
memset(addr, 0, sizeof(*addr));
+ if ((cssid = virXMLPropUInt(node, "cssid", 0, VIR_XML_PROP_OPTIONAL, + &addr->cssid)) < 0) + return -1; + + if ((ssid = virXMLPropUInt(node, "ssid", 0, VIR_XML_PROP_OPTIONAL, + &addr->ssid)) < 0) + return -1; + + if ((devno = virXMLPropUInt(node, "devno", 0, VIR_XML_PROP_OPTIONAL, + &addr->devno)) < 0) + return -1; + + if (!virDomainDeviceCCWAddressIsValid(addr)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid specification for virtio ccw address: cssid='%u' ssid='%u' devno='%u'"), + addr->cssid, addr->ssid, addr->devno);
This misrepresents user's input and could even be confusing. While the parser uses 0 for number base autodetection, libvirt outputs the values in hex format. (See the appropriate case in virDomainDeviceInfoFormat) Reporting them as base 10 numbers could lead to a red herring chase.
+ return -1; + }
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 621ff1b476..5a98cd8c0f 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -316,40 +316,23 @@ int virDomainDeviceDriveAddressParseXML(xmlNodePtr node, virDomainDeviceDriveAddress *addr) { - g_autofree char *controller = virXMLPropString(node, "controller"); - g_autofree char *bus = virXMLPropString(node, "bus"); - g_autofree char *target = virXMLPropString(node, "target"); - g_autofree char *unit = virXMLPropString(node, "unit"); - memset(addr, 0, sizeof(*addr)); - if (controller && - virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'controller' attribute")); + if (virXMLPropUInt(node, "controller", 10, VIR_XML_PROP_OPTIONAL, + &addr->controller) < 0) return -1; - } - if (bus && - virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'bus' attribute")); + if (virXMLPropUInt(node, "bus", 10, VIR_XML_PROP_OPTIONAL, + &addr->bus) < 0) return -1; - } - if (target && - virStrToLong_uip(target, NULL, 10, &addr->target) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'target' attribute")); + if (virXMLPropUInt(node, "target", 10, VIR_XML_PROP_OPTIONAL, + &addr->target) < 0) return -1; - } - if (unit && - virStrToLong_uip(unit, NULL, 10, &addr->unit) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'unit' attribute")); + if (virXMLPropUInt(node, "unit", 10, VIR_XML_PROP_OPTIONAL, + &addr->unit) < 0) return -1; - } return 0; } -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 5a98cd8c0f..587f966398 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -341,32 +341,19 @@ int virDomainDeviceVirtioSerialAddressParseXML(xmlNodePtr node, virDomainDeviceVirtioSerialAddress *addr) { - g_autofree char *controller = virXMLPropString(node, "controller"); - g_autofree char *bus = virXMLPropString(node, "bus"); - g_autofree char *port = virXMLPropString(node, "port"); - memset(addr, 0, sizeof(*addr)); - if (controller && - virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'controller' attribute")); + if (virXMLPropUInt(node, "controller", 10, VIR_XML_PROP_OPTIONAL, + &addr->controller) < 0) return -1; - } - if (bus && - virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'bus' attribute")); + if (virXMLPropUInt(node, "bus", 10, VIR_XML_PROP_OPTIONAL, + &addr->bus) < 0) return -1; - } - if (port && - virStrToLong_uip(port, NULL, 10, &addr->port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'port' attribute")); + if (virXMLPropUInt(node, "port", 10, VIR_XML_PROP_OPTIONAL, + &addr->port) < 0) return -1; - } return 0; } -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 587f966398..f7bf2de6f1 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -362,24 +362,15 @@ int virDomainDeviceCcidAddressParseXML(xmlNodePtr node, virDomainDeviceCcidAddress *addr) { - g_autofree char *controller = virXMLPropString(node, "controller"); - g_autofree char *slot = virXMLPropString(node, "slot"); - memset(addr, 0, sizeof(*addr)); - if (controller && - virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'controller' attribute")); + if (virXMLPropUInt(node, "controller", 10, VIR_XML_PROP_OPTIONAL, + &addr->controller) < 0) return -1; - } - if (slot && - virStrToLong_uip(slot, NULL, 10, &addr->slot) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'slot' attribute")); + if (virXMLPropUInt(node, "slot", 10, VIR_XML_PROP_OPTIONAL, + &addr->slot) < 0) return -1; - } return 0; } -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index f7bf2de6f1..1ce31e9234 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -403,19 +403,16 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node, virDomainDeviceUSBAddress *addr) { g_autofree char *port = virXMLPropString(node, "port"); - g_autofree char *bus = virXMLPropString(node, "bus"); memset(addr, 0, sizeof(*addr)); if (port && virDomainDeviceUSBAddressParsePort(addr, port) < 0) return -1; - if (bus && - virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'bus' attribute")); + if (virXMLPropUInt(node, "bus", 10, VIR_XML_PROP_OPTIONAL, + &addr->bus) < 0) return -1; - } + return 0; } -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 1ce31e9234..a2893fec6f 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -468,28 +468,14 @@ int virInterfaceLinkParseXML(xmlNodePtr node, virNetDevIfLink *lnk) { - int state; - - g_autofree char *stateStr = virXMLPropString(node, "state"); - g_autofree char *speedStr = virXMLPropString(node, "speed"); - - if (stateStr) { - if ((state = virNetDevIfStateTypeFromString(stateStr)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown link state: %s"), - stateStr); - return -1; - } - lnk->state = state; - } + if (virXMLPropEnum(node, "state", virNetDevIfStateTypeFromString, + VIR_XML_PROP_OPTIONAL, &lnk->state) < 0) + return -1; - if (speedStr && - virStrToLong_ui(speedStr, NULL, 10, &lnk->speed) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Unable to parse link speed: %s"), - speedStr); + if (virXMLPropUInt(node, "speed", 10, VIR_XML_PROP_OPTIONAL, + &lnk->speed) < 0) return -1; - } + return 0; } -- 2.26.2
On Fri, Apr 16, 2021 at 14:21:00 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 1ce31e9234..a2893fec6f 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -468,28 +468,14 @@ int virInterfaceLinkParseXML(xmlNodePtr node, virNetDevIfLink *lnk) { - int state; - - g_autofree char *stateStr = virXMLPropString(node, "state"); - g_autofree char *speedStr = virXMLPropString(node, "speed"); - - if (stateStr) { - if ((state = virNetDevIfStateTypeFromString(stateStr)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown link state: %s"), - stateStr); - return -1; - } - lnk->state = state; - } + if (virXMLPropEnum(node, "state", virNetDevIfStateTypeFromString, + VIR_XML_PROP_OPTIONAL, &lnk->state) < 0) + return -1;
- if (speedStr && - virStrToLong_ui(speedStr, NULL, 10, &lnk->speed) < 0) {
This was using virStrToLong_ui which has the 'wrap-around' anti feature.
- virReportError(VIR_ERR_XML_ERROR, - _("Unable to parse link speed: %s"), - speedStr); + if (virXMLPropUInt(node, "speed", 10, VIR_XML_PROP_OPTIONAL, + &lnk->speed) < 0)
Strictifying the parser must be sufficiently justified in the commit message.
return -1; - } + return 0; }
-- 2.26.2
On Fri, Apr 16, 2021 at 14:20:50 +0200, Tim Wiederhake wrote:
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Tim Wiederhake (10): virDomainBackupDiskDefParseXML: Use virXMLProp* virDomainBackupDefParse: Use virXMLProp* virZPCIDeviceAddressParseXML: Use virXMLProp* virPCIDeviceAddressParseXML: Use virXMLProp* virDomainDeviceCCWAddressParseXML: Use virXMLProp* virDomainDeviceDriveAddressParseXML: Use virXMLProp* virDomainDeviceVirtioSerialAddressParseXML: Use virXMLProp* virDomainDeviceCcidAddressParseXML: Use virXMLProp* virDomainDeviceUSBAddressParseXML: Use virXMLProp* virInterfaceLinkParseXML: Use virXMLProp*
Once you deal with my feedback and preferably rebase on top of the two patches I've sent for fixing the virXMLProp flags Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Peter Krempa -
Tim Wiederhake