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

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



On 2012年07月25日 17:41, Peter Krempa wrote:
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)",

One space nit. ACK.

Regards,
Osier


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