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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/