[libvirt] [PATCH] virsh: Clean up and refactor cmdDomDisplay() and fix some possible leaks

[Peter Krempa]

This patch refactors cmdDomDisplay to update coding style and remove
some possible memory leaks on error paths. This patch also fixes flag
variable type to unsigned.
---
 tools/virsh.c |   76 ++++++++++++++++++++++++++------------------------------
 1 files changed, 35 insertions(+), 41 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 6d65036..eea580d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13861,16 +13861,15 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
     virDomainPtr dom;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     bool ret = false;
-    char *doc;
-    char *xpath;
-    char *listen_addr;
+    char *doc = NULL;
+    char *xpath = NULL;
+    char *listen_addr = NULL;
     int port, tls_port = 0;
     char *passwd = NULL;
     char *output = NULL;
     const char *scheme[] = { "vnc", "spice", "rdp", NULL };
     int iter = 0;
-    int tmp;
-    int flags = 0;
+    unsigned int flags = 0;

     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;
@@ -13886,40 +13885,34 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptBool(cmd, "include-password"))
         flags |= VIR_DOMAIN_XML_SECURE;

-    doc = virDomainGetXMLDesc(dom, flags);
-
-    if (!doc)
+    if (!(doc = virDomainGetXMLDesc(dom, flags)))
         goto cleanup;

-    xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt);
-    VIR_FREE(doc);
-    if (!xml)
+    if (!(xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt)))
         goto cleanup;

     /* Attempt to grab our display info */
     for (iter = 0; scheme[iter] != NULL; iter++) {
         /* Create our XPATH lookup for the current display's port */
-        virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']"
-                "/@port)", scheme[iter]);
-        if (!xpath) {
+        VIR_FREE(xpath);
+        if (virAsprintf(&xpath,
+                        "string(/domain/devices/graphics[@type='%s']/@port)",
+                        scheme[iter]) < 0) {
             virReportOOMError();
             goto cleanup;
         }

-        /* Attempt to get the port number for the current graphics scheme */
-        tmp = virXPathInt(xpath, ctxt, &port);
-        VIR_FREE(xpath);
-
-        /* If there is no port number for this type, then jump to the next
-         * scheme
+        /* Attempt to get the port number for the current graphics scheme.
+         * If there is no port number for this type, then jump to the next scheme.
          */
-        if (tmp)
+        if (virXPathInt(xpath, ctxt, &port) < 0)
             continue;

         /* Create our XPATH lookup for the current display's address */
-        virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']"
-                "/@listen)", scheme[iter]);
-        if (!xpath) {
+        VIR_FREE(xpath);
+        if (virAsprintf(&xpath,
+                        "string(/domain/devices/graphics[@type='%s']/@listen)",
+                        scheme[iter]) < 0) {
             virReportOOMError();
             goto cleanup;
         }
@@ -13928,7 +13921,6 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
          * graphics scheme
          */
         listen_addr = virXPathString(xpath, ctxt);
-        VIR_FREE(xpath);

         /* Per scheme data mangling */
         if (STREQ(scheme[iter], "vnc")) {
@@ -13936,31 +13928,32 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
             port -= 5900;
         } else if (STREQ(scheme[iter], "spice")) {
             /* Create our XPATH lookup for the SPICE TLS Port */
-            virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']"
-                    "/@tlsPort)", scheme[iter]);
-            if (!xpath) {
+            VIR_FREE(xpath);
+            if (virAsprintf(&xpath,
+                            "string(/domain/devices/graphics[@type='%s']"
+                             "/@tlsPort)",
+                             scheme[iter]) < 0) {
                 virReportOOMError();
                 goto cleanup;
             }

             /* Attempt to get the TLS port number for SPICE */
-            tmp = virXPathInt(xpath, ctxt, &tls_port);
-            VIR_FREE(xpath);
-            if (tmp)
+            if (virXPathInt(xpath, ctxt, &tls_port) < 0)
                 tls_port = 0;

             if (vshCommandOptBool(cmd, "include-password")) {
                 /* Create our XPATH lookup for the SPICE password */
-                virAsprintf(&xpath, "string(/domain/devices/graphics"
-                        "[@type='%s']/@passwd)", scheme[iter]);
-                if (!xpath) {
+                VIR_FREE(xpath);
+                if (virAsprintf(&xpath,
+                                "string(/domain/devices/graphics"
+                                "[@type='%s']/@passwd)",
+                                scheme[iter]) < 0) {
                     virReportOOMError();
                     goto cleanup;
                 }

                 /* Attempt to get the SPICE password */
                 passwd = virXPathString(xpath, ctxt);
-                VIR_FREE(xpath);
             }
         }

@@ -13968,12 +13961,11 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
         virBufferAsprintf(&buf, "%s://", scheme[iter]);

         /* Then host name or IP */
-        if (!listen_addr || STREQ((const char *)listen_addr, "0.0.0.0"))
+        if (!listen_addr || STREQ(listen_addr, "0.0.0.0"))
             virBufferAddLit(&buf, "localhost");
         else
             virBufferAsprintf(&buf, "%s", listen_addr);

-        VIR_FREE(listen_addr);

         /* Add the port */
         if (STREQ(scheme[iter], "spice"))
@@ -13986,10 +13978,8 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
             virBufferAsprintf(&buf, "&tls-port=%d", tls_port);

         /* Password */
-        if (passwd) {
+        if (passwd)
             virBufferAsprintf(&buf, "&password=%s", passwd);
-            VIR_FREE(passwd);
-        }

         /* Ensure we can print our URI */
         if (virBufferError(&buf)) {
@@ -14000,7 +13990,6 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
         /* Print out our full URI */
         output = virBufferContentAndReset(&buf);
         vshPrint(ctl, "%s", output);
-        VIR_FREE(output);

         /* We got what we came for so return successfully */
         ret = true;
@@ -14008,6 +13997,11 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
     }

 cleanup:
+    VIR_FREE(doc);
+    VIR_FREE(xpath);
+    VIR_FREE(listen_addr);
+    VIR_FREE(passwd);
+    VIR_FREE(output);
     xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
     virDomainFree(dom);
-- 
1.7.8.6