On 02/14/2013 09:42 AM, John Ferlan wrote:
--- tests/commandtest.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c index 93c6333..3bfc358 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -680,7 +680,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED) goto cleanup; }
- if (!outbuf || *outbuf) { + if (*outbuf) { puts("output buffer is not an allocated empty string"); goto cleanup; } @@ -702,7 +702,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED) goto cleanup; }
- if (!outbuf || *outbuf || !errbuf || *errbuf) { + if (*outbuf || *errbuf) { puts("output buffers are not allocated empty strings"); goto cleanup;
This part seems reasonable.
} @@ -936,6 +936,7 @@ mymain(void) int fd; virCommandTestDataPtr test = NULL; int timer = -1; + int virinitret;
if (virThreadInitialize() < 0) return EXIT_FAILURE; @@ -963,16 +964,19 @@ mymain(void) dup2(fd, 6) < 0 || dup2(fd, 7) < 0 || dup2(fd, 8) < 0 || - (fd > 8 && VIR_CLOSE(fd) < 0)) + (fd > 8 && VIR_CLOSE(fd) < 0)) { + VIR_FORCE_CLOSE(fd); return EXIT_FAILURE;
Odd that Coverity complains about the leak of fd, but not about the leak of all the other dup2'd fds; and at any rate, the leak is inconsequential since we are about to exit. But if this shuts things up, I'm okay with it.
+ } + sa_assert(fd == -1);
Yes, this is true, and if it helps Coverity, I'm fine with it.
/* Prime the debug/verbose settings from the env vars, * since we're about to reset 'environ' */ ignore_value(virTestGetDebug()); ignore_value(virTestGetVerbose());
- if (virInitialize() < 0) - return EXIT_FAILURE; + /* Make sure to not leak fd's */ + virinitret = virInitialize();
/* Phase two of killing interfering fds; see above. */ fd = 3; @@ -988,6 +992,9 @@ mymain(void) fd = 8; VIR_FORCE_CLOSE(fd);
+ if (virinitret < 0) + return EXIT_FAILURE;
Seems okay to me. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org