Re: [PATCH v2] parallels: Resolve some resource leaks

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

 



On 01/23/2013 08:19 AM, Peter Krempa wrote:
> On 01/22/13 21:10, John Ferlan wrote:
>> Be sure to VIR_FREE(accel) and moved virDomainVideoDefFree() within
>> no_memory
>> label to be consistent
>>
>> Resolve resource leak in parallelsApplyIfaceParams() when the 'oldnet' is
>> allocated locally. Also virCommandFree(cmd) as necessary.
>>
>> ---
>>
>> This is a followup patch to:
>>
>> https://www.redhat.com/archives/libvir-list/2013-January/msg01565.html
>>
>> Changes in v2:
>>     Change label from cleanup to error
>>     Add calls to virCommandFree()
>>
>>   src/parallels/parallels_driver.c | 40
>> +++++++++++++++++++++++++---------------
>>   1 file changed, 25 insertions(+), 15 deletions(-)
>>
> 
> [...]
> 
>>       }
>>
>> @@ -1901,15 +1902,24 @@ static int
>> parallelsApplyIfaceParams(parallelsDomObjPtr pdom,
>>           is_changed = true;
>>       }
>>
>> +    if (create)
>> +        VIR_FREE(oldnet);
>> +
>>       if (!create && !is_changed) {
>>           /* nothing changed - no need to run prlctl */
>> +        virCommandFree(cmd);
>>           return 0;
>>       }
>>
>>       if (virCommandRun(cmd, NULL))
>> -        return -1;
>> +        goto error;
>>
>>       return 0;
> 
> Um, I probably wasn't clear enough. The virCommand cmd needs to be freed
> also on success.
> 

I had checked a couple of other callers "at random" and naturally the
ones I looked at didn't free it on success.  The followup could be a bit
larger.

>> +error:
> 
> I'd go for a cleanup label here and add a "ret" variable to indicate
> success or failure.
> 
>> +    if (create)
>> +        VIR_FREE(oldnet);
>> +    virCommandFree(cmd);
>> +    return -1;
>>   }
>>
>>   static int
>>
> 
> 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]