[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]