From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 19 Oct 2022 11:59:17 +0000 (+0200)
Subject: virshFindDisk: Sanitize use of 'tmp' variable
X-Git-Tag: v9.0.0-rc1~229
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=22766a1a5371ce20579c503363e0d9b27091e393;p=thirdparty%2Flibvirt.git

virshFindDisk: Sanitize use of 'tmp' variable

The return value of virXMLPropString was assigned into 'tmp' multiple
times and to prevent static analyzers moaning about a potential leak a
short-circuited if logic or was used.

Replace the code by having a helper variable for each possibility and
also replace the for-loop to iterate elements.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5943c31159..2d162cf8c0 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12642,7 +12642,6 @@ virshFindDisk(const char *doc,
     g_autoptr(xmlXPathContext) ctxt = NULL;
     g_autofree xmlNodePtr *nodes = NULL;
     ssize_t nnodes;
-    xmlNodePtr cur = NULL;
     size_t i;
 
     xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt);
@@ -12658,6 +12657,13 @@ virshFindDisk(const char *doc,
 
     /* search disk using @path */
     for (i = 0; i < nnodes; i++) {
+        xmlNodePtr sourceNode;
+        g_autofree char *sourceFile = NULL;
+        g_autofree char *sourceDev = NULL;
+        g_autofree char *sourceDir = NULL;
+        g_autofree char *sourceName = NULL;
+        g_autofree char *targetDev = NULL;
+
         if (type == VIRSH_FIND_DISK_CHANGEABLE) {
             g_autofree char *device = virXMLPropString(nodes[i], "device");
 
@@ -12668,29 +12674,25 @@ virshFindDisk(const char *doc,
                 continue;
         }
 
-        cur = nodes[i]->children;
-        while (cur != NULL) {
-            if (cur->type == XML_ELEMENT_NODE) {
-                g_autofree char *tmp = NULL;
-
-                if (virXMLNodeNameEqual(cur, "source")) {
-                    if ((tmp = virXMLPropString(cur, "file")) ||
-                        (tmp = virXMLPropString(cur, "dev")) ||
-                        (tmp = virXMLPropString(cur, "dir")) ||
-                        (tmp = virXMLPropString(cur, "name"))) {
-                    }
-                } else if (virXMLNodeNameEqual(cur, "target")) {
-                    tmp = virXMLPropString(cur, "dev");
-                }
+        if ((sourceNode = virXMLNodeGetSubelement(nodes[i], "source"))) {
+            sourceFile = virXMLPropString(sourceNode, "file");
+            sourceDev = virXMLPropString(sourceNode, "dev");
+            sourceDir = virXMLPropString(sourceNode, "dir");
+            sourceName = virXMLPropString(sourceNode, "name");
+        }
 
-                if (STREQ_NULLABLE(tmp, path)) {
-                    xmlNodePtr ret = xmlCopyNode(nodes[i], 1);
-                    /* drop backing store since they are not needed here */
-                    virshDiskDropBackingStore(ret);
-                    return ret;
-                }
-            }
-            cur = cur->next;
+        ctxt->node = nodes[i];
+        targetDev = virXPathString("string(./target/@dev)", ctxt);
+
+        if (STREQ_NULLABLE(targetDev, path) ||
+            STREQ_NULLABLE(sourceFile, path) ||
+            STREQ_NULLABLE(sourceDev, path) ||
+            STREQ_NULLABLE(sourceDir, path) ||
+            STREQ_NULLABLE(sourceName, path)) {
+            xmlNodePtr ret = xmlCopyNode(nodes[i], 1);
+            /* drop backing store since they are not needed here */
+            virshDiskDropBackingStore(ret);
+            return ret;
         }
     }