Re: [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)",