[libvirt] [PATCH 0/6] Fix some memory leaks and other issues find by coverity tool

Pavel Hrdina phrdina at redhat.com
Tue Jan 14 09:06:17 UTC 2014


On 13.1.2014 18:17, John Ferlan wrote:
>
>
> On 01/13/2014 11:12 AM, Pavel Hrdina wrote:
>> This patch series fixes few memory leaks found by coverity tool
>> to make that tool happy.
>>
>> The last patch is adding only one comment to hide "double_close" error as
>> coverity tool is wrong about this and we don't have to see it in every report.
>> Check the patch for more information.
>>
>> Pavel Hrdina (6):
>>    Fix memory leak in openvz_conf.c
>>    Fix possible memory leak in phyp_driver.c
>>    Fix possible memory leak in util/virxml.c
>>    Fix possible memory leak in virsh-domain-monitor.c
>>    Fix memory leak in securityselinuxlabeltest.c
>>    Fix coverity complain in commandtest.c
>>
>>   src/openvz/openvz_conf.c         | 5 +++--
>>   src/phyp/phyp_driver.c           | 4 +++-
>>   src/util/virxml.c                | 2 ++
>>   tests/commandtest.c              | 1 +
>>   tests/securityselinuxlabeltest.c | 5 ++++-
>>   tools/virsh-domain-monitor.c     | 4 ++++
>>   6 files changed, 17 insertions(+), 4 deletions(-)
>>
>
> hrmph... for some reason 1/6 and 4/6 haven't made it to my inbox yet.  I
> hope the Red Hat mail filter isn't acting up again...
>
> Anyway
>
> 1/6 is an ACK
>
> 4/6 I think still issues
>
> First off 'type' and 'device' need to be initialized to NULL, then don't
> worry about the "if details()" within the (!target) condition.
>
> Second either we have to choose to exit if the virXPathString() fails
> for type/device or when we go to vshPrint() there needs to be a "type ?
> type : "-"" and "device ? device : "-"" in order to ensure we're not
> printing garbage or old data
>
> Not sure which of the methods is better if we fail to alloc type or
> device is better.  Note that virXPathString() can return NULL for more
> reasons than just allocation failure.
>

Hi John,

Thanks for the review, if I'll obtain git write access I'll bush the
ACKed patches by myself with the modifications or I'll ask someone to
do it.

> Since Osier wrote the code, maybe he can help decide the course of
> action to take.  Is it better to display "-" or just fail?  On an
> allocation failure, then virXPathString for target will probably fail
> too landing us in that error path, so we could just take that option too!
>

Hi Osier,

Let me know what you think is the better way.

Pavel

> John
>




More information about the libvir-list mailing list