[libvirt] [PATCH v2] virsh: Rewrite cmdDomDisplay

[Martin Kletzander]

On 11/28/2012 02:38 PM, Peter Krempa wrote:
> On 11/22/12 16:24, Martin Kletzander wrote:
[...]
>> +        /* We can query this info for all the graphics types since we'll
>> +         * get nothing for the unsupported ones (just rdp for now).
>> +         * Also the parameter '--include-password' was already taken
>> +         * care of when getting the XML */
>> +
>> +        /* Create our XPATH lookup for the password */
>> +        if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "passwd") < 0)
>> +            goto no_memory;
>> +
>> +        /* Attempt to get the password */
>> +        passwd = virXPathString(xpath, ctxt);
> 
> You forgot to VIR_FREE(xpath) here leaking one of the query strings.
> 
>> +
>> +        if (STREQ(scheme[iter], "vnc"))
>> +            /* VNC protocol handlers take their port number as
>> +             * 'port' - 5900 */
>>               port -= 5900;
>> -        } else if (STREQ(scheme[iter], "spice")) {
>> -            /* Create our XPATH lookup for the SPICE TLS Port */
>> -            virAsprintf(&xpath,
[...]
> 
> Nice cleanup. ACK with the memleak fixed.
> 
> Peter
> 

I fixed the leak and added curly braces around "multi-line" block after
that.  Long story short, I pushed it with the following squashed.

Martin

---
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1e8ccc9..1723413 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7068,11 +7068,13 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)

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

-        if (STREQ(scheme[iter], "vnc"))
+        if (STREQ(scheme[iter], "vnc")) {
             /* VNC protocol handlers take their port number as
              * 'port' - 5900 */
             port -= 5900;
+        }

         /* Create our XPATH lookup for TLS Port (automatically skipped
          * for unsupported schemes */