[PATCH 0/3] Resolved recent/new Coverity warnings
For virlog.c and commandtest.c - it seems recent changes within the modules have allowed Coverity to dig deeper and find different issues. Not sure why Coverity has that "ability"... Since I was making changes, figured I'd clean up the lxc_controller fd leak as well - these should clean the Jenkins build environment. John Ferlan (3): Coverity: Resolve a CHECKED_RETURN message Coverity: Resolve a FORWARD_NULL Coverity: Resolve a RESOURCE_LEAK src/lxc/lxc_controller.c | 14 ++++++++++---- src/util/virlog.c | 2 +- tests/commandtest.c | 1 + 3 files changed, 12 insertions(+), 5 deletions(-) -- 1.8.5.3
Recent changes to the module seemed to have caused Coverity to find a new issue regarding the failure to check the return from a sendmsg. The code doesn't seem to care about the return status, so just added an ignore_value to keep Coverity quiet. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virlog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index d0afd10..056950e 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1003,7 +1003,7 @@ virLogOutputToJournald(virLogSourcePtr source, mh.msg_controllen = cmsg->cmsg_len; - sendmsg(journalfd, &mh, MSG_NOSIGNAL); + ignore_value(sendmsg(journalfd, &mh, MSG_NOSIGNAL)); cleanup: VIR_LOG_CLOSE(buffd); -- 1.8.5.3
Recent changes in the module seemed to have caused Coverity to reanalyze certain parts of the code. Previously the code was modified via commit id '11a11812' to resolve a different error (perhaps DEADCODE). Up through commit id '7b3f1f8c' there were no issues. The new error indicats the 'outbuf' was checked for NULL and then complains because of the dereference. Adding checks for non-NULL prior to the deref resulted in a DEADCODE message. So, resolve using an sa_assert() to keep Coverity quiet especially since it doesn't understand that outbuf will change as a result of a successful virCommandRun() call. Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/commandtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/commandtest.c b/tests/commandtest.c index c5c3a9a..7d2161c 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -690,6 +690,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED) goto cleanup; } + sa_assert(outbuf); if (*outbuf) { puts("output buffer is not an allocated empty string"); goto cleanup; -- 1.8.5.3
On error the lofd would have been leaked. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/lxc/lxc_controller.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 800a306..928a43d 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -383,6 +383,7 @@ static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDefPtr disk) int lofd; char *loname = NULL; const char *src = virDomainDiskGetSource(disk); + int ret = -1; if ((lofd = virFileLoopDeviceAssociate(src, &loname)) < 0) return -1; @@ -395,13 +396,18 @@ static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDefPtr disk) * the rest of container setup 'just works' */ virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); - if (virDomainDiskSetSource(disk, loname) < 0) { - VIR_FREE(loname); - return -1; - } + if (virDomainDiskSetSource(disk, loname) < 0) + goto cleanup; + + ret = 0; + + cleanup: VIR_FREE(loname); + if (ret == -1) + VIR_FORCE_CLOSE(lofd); return lofd; + } -- 1.8.5.3
Ignore the old man with bad eyesight who picked the wrong line in his command cheat sheet file... This was supposed to go to libvir-list (sigh)... John
participants (1)
-
John Ferlan