]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
conf: refactor virDomainBlkioDeviceParseXML to reduce calls to xmlNodeGetContent
authorLaine Stump <laine@redhat.com>
Fri, 19 Jun 2020 01:52:38 +0000 (21:52 -0400)
committerLaine Stump <laine@redhat.com>
Wed, 5 Aug 2020 04:00:18 +0000 (00:00 -0400)
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 <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
src/conf/domain_conf.c

index a6d23a2238d833b3805a042421f4719b2c4ffc80..251db11df9d97ddcd7c38cf4f602d742c05ff5b3 100644 (file)
@@ -1642,73 +1642,85 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root,
                              virBlkioDevicePtr dev)
 {
     xmlNodePtr node;
-    g_autofree char *c = 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);
+    g_autofree char *path = NULL;
+
+    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;
 }