[libvirt] [PATCH v2 9/9] reworked cpu-baseline command for virsh

[Daniel Veillard]

On Wed, Feb 17, 2010 at 12:40:49PM +0100, Jiri Denemark wrote:
> > diff --git a/tools/virsh.c b/tools/virsh.c
> > index 7db48d9..95f5801 100644
> > --- a/tools/virsh.c
> > +++ b/tools/virsh.c
> > @@ -7025,6 +7026,121 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
> ...
> > +static int
> > +cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
> > +{
> ...
> > +    for (i = 0;i < obj->nodesetval->nodeNr;i++) {
> > +        buf = xmlBufferCreate();
> > +        if (buf == NULL)
> > +            goto no_memory;
> > +        sctxt = xmlSaveToBuffer(buf, NULL, 0);
> > +        if (sctxt == NULL)
> Hmm, we would leak buf here, wouldn't we?

  Ah right but I prefer to free it here before the goto

> > +            goto no_memory;
> > +
> > +        xmlSaveTree(sctxt, obj->nodesetval->nodeTab[i]);
> > +        xmlSaveClose(sctxt);
> > +
> > +        list = vshRealloc(ctl, list, sizeof(char *) * (count + 1));
> > +        list[count++] = (char *) buf->content;
> > +        buf->content = NULL;

actually there is an 
             xmlBufferFree(buf);

here in my patch, it's needed to not leak in the loop.

> > +        buf = NULL;
> > +    }
> > +
> > +    if (count == 0) {
> > +        vshError(ctl, _("No host CPU specified in '%s'"), from);
> > +        ret = FALSE;
> > +        goto cleanup;
> > +    }
> > +
> > +    result = virConnectBaselineCPU(ctl->conn, list, count, 0);
> > +
> > +    if (result)
> > +        vshPrint(ctl, "%s", result);
> > +    else
> > +        ret = FALSE;
> > +
> > +cleanup:
> > +    xmlXPathFreeObject(obj);
> > +    xmlXPathFreeContext(ctxt);
> > +    xmlFreeDoc(doc);
> > +    VIR_FREE(result);
> > +    if ((list != NULL) && (count > 0)) {
> > +        for (i = 0;i < count;i++)
> > +            VIR_FREE(list[i]);
> > +    }
> > +    VIR_FREE(list);
> > +    VIR_FREE(buffer);
> 
> This would fix the leak:
>        xmlBufferFree(buf);

 I prefer to do it right before the goto instead.

> > +    return ret;
> > +
> > +no_memory:
> > +    vshError(ctl, "%s", _("Out of memory"));
> > +    ret = FALSE;
> > +}
> 
> Except for the leak on error path, the patch looks good. ACK for the fixed
> version.

  Okay, thanks, I pushed this !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/