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

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