Re: [PATCH] conf: Fix memory leaks in virStoragePoolDefParseSource

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

 



On 05/09/2012 10:45 AM, Alex Jia wrote:
On 05/09/2012 04:06 PM, Peter Krempa wrote:
The problem with the added line is not visible with this context, so
I'm adding
some more:

source->nhost = virXPathNodeSet("./host", ctxt,&nodeset);

if (source->nhost) {
if (VIR_ALLOC_N(source->hosts, source->nhost)< 0) {
virReportOOMError();
goto cleanup;
}

for (i = 0 ; i< source->nhost ; i++) {
name = virXMLPropString(nodeset[i], "name");
if (name == NULL) {
virStorageReportError(VIR_ERR_XML_ERROR,
"%s", _("missing storage pool host name"));
goto cleanup;
}
source->hosts[i].name = name;

port = virXMLPropString(nodeset[i], "port");
if (port) {
if (virStrToLong_i(port, NULL, 10,&source->hosts[i].port)< 0) {
virStorageReportError(VIR_ERR_XML_ERROR,
_("Invalid port number: %s"),
port);
goto cleanup;
}
}
+ VIR_FREE(nodeset);
}
}

You added the VIR_FREE inside the for loop, so it gets freed before
the next iteration
and might cause a segfault in the second iteration.
Although I haven't meet a segfault error, I think you're right, thanks.

You were lucky and the loop iterated just once. If the xml contained more "host" elements you'd hit the SIGSEGV. The correct point to free the nodeset is _after_ the for loop.

Peter

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]