On 14/01/14 01: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
Both the "type" and "device" are not mandatory attributes for disk
device, "type" defaults to "file", and "device" defaults to "disk". So in
principle they can't be NULL, if virXPathString returns NULL for them,
it means either the internal data structure is broken (unlikely but
critical), or virXPathString itself had some funny things. For either
of them, we should fail. It's unlike "source" element, since the disk
source can be empty, e.g. for a Floppy drive.
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.
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!
Regards,
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list