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

[Alex Jia]

On 05/09/2012 05:50 PM, Peter Krempa wrote:
> 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.
Indeed, thanks.
>
>
> Peter
>