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

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

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...


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.



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