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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to