From b595f44525412bb7e5b02a57faa442b02cc85f57 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Thu, 18 Jun 2020 21:52:38 -0400 Subject: [PATCH] conf: refactor virDomainBlkioDeviceParseXML to reduce calls to xmlNodeGetContent virDomainBlkioDeviceParseXML() calls xmlNodeGetContent() multiple times in a loop, but can easily be refactored to call it once for all element nodes, and then use the result of that one call in each of the (mutually exclusive) blocks that previously each had their own call to xmlNodeGetContent. This is being done in order to reduce the number of changes needed in an upcoming patch that will eliminate the lack of checking for NULL on return from xmlNodeGetContent(). As part of the simplification, the while() loop has been changed into a for() so that we can use "continue" without bypassing the "node = node->next". Signed-off-by: Laine Stump Reviewed-by: Michal Privoznik --- src/conf/domain_conf.c | 128 ++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 58 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a6d23a2238..251db11df9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1642,73 +1642,85 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root, virBlkioDevicePtr dev) { xmlNodePtr node; - g_autofree char *c = NULL; + g_autofree char *path = NULL; - node = root->children; - while (node) { - if (node->type == XML_ELEMENT_NODE) { - if (virXMLNodeNameEqual(node, "path") && !dev->path) { - dev->path = (char *)xmlNodeGetContent(node); - } else if (virXMLNodeNameEqual(node, "weight")) { - c = (char *)xmlNodeGetContent(node); - if (virStrToLong_ui(c, NULL, 10, &dev->weight) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("could not parse weight %s"), - c); - goto error; - } - VIR_FREE(c); - } else if (virXMLNodeNameEqual(node, "read_bytes_sec")) { - c = (char *)xmlNodeGetContent(node); - if (virStrToLong_ull(c, NULL, 10, &dev->rbps) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("could not parse read bytes sec %s"), - c); - goto error; - } - VIR_FREE(c); - } else if (virXMLNodeNameEqual(node, "write_bytes_sec")) { - c = (char *)xmlNodeGetContent(node); - if (virStrToLong_ull(c, NULL, 10, &dev->wbps) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("could not parse write bytes sec %s"), - c); - goto error; - } - VIR_FREE(c); - } else if (virXMLNodeNameEqual(node, "read_iops_sec")) { - c = (char *)xmlNodeGetContent(node); - if (virStrToLong_ui(c, NULL, 10, &dev->riops) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("could not parse read iops sec %s"), - c); - goto error; - } - VIR_FREE(c); - } else if (virXMLNodeNameEqual(node, "write_iops_sec")) { - c = (char *)xmlNodeGetContent(node); - if (virStrToLong_ui(c, NULL, 10, &dev->wiops) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("could not parse write iops sec %s"), - c); - goto error; - } - VIR_FREE(c); - } + for (node = root->children; node != NULL; node = node->next) { + g_autofree char *c = NULL; + + if (node->type != XML_ELEMENT_NODE) + continue; + + c = (char *)xmlNodeGetContent(node); + + if (virXMLNodeNameEqual(node, "path")) { + /* To avoid the need for explicit cleanup on failure, + * don't set dev->path until we're assured of + * success. Until then, store it in an autofree pointer. + */ + if (!path) + path = g_steal_pointer(&c); + continue; + } + + if (virXMLNodeNameEqual(node, "weight")) { + if (virStrToLong_ui(c, NULL, 10, &dev->weight) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse weight %s"), + c); + return -1; + } + continue; + } + + if (virXMLNodeNameEqual(node, "read_bytes_sec")) { + if (virStrToLong_ull(c, NULL, 10, &dev->rbps) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse read bytes sec %s"), + c); + return -1; + } + continue; + } + + if (virXMLNodeNameEqual(node, "write_bytes_sec")) { + if (virStrToLong_ull(c, NULL, 10, &dev->wbps) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse write bytes sec %s"), + c); + return -1; + } + continue; + } + + if (virXMLNodeNameEqual(node, "read_iops_sec")) { + if (virStrToLong_ui(c, NULL, 10, &dev->riops) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse read iops sec %s"), + c); + return -1; + } + continue; + } + + if (virXMLNodeNameEqual(node, "write_iops_sec")) { + if (virStrToLong_ui(c, NULL, 10, &dev->wiops) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse write iops sec %s"), + c); + return -1; + } + continue; } - node = node->next; } - if (!dev->path) { + + if (!path) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("missing per-device path")); return -1; } + dev->path = g_steal_pointer(&path); return 0; - - error: - VIR_FREE(dev->path); - return -1; }