[PATCHv2 2/2] tests: fix leaks in commandtest

[Eric Blake <eblake@xxxxxxxxxx>]

Most leaks could only occur on error cleanup paths.
---

Respin of Jiri's 2/4 v1 patch, now much smaller by relying
on guaranteed semantics.

Jiri's patch 1 and 4 of the v1 series are still okay to apply
as-is, but patch 3/4 needs rebasing onto this patch.

 tests/commandtest.c |   72 +++++++++++++++++++++++++++++++-------------------
 1 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/tests/commandtest.c b/tests/commandtest.c
index ace2f33..2666d61 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -153,6 +153,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -164,6 +165,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -181,6 +183,7 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) {
     int newfd1 = dup(STDERR_FILENO);
     int newfd2 = dup(STDERR_FILENO);
     int newfd3 = dup(STDERR_FILENO);
+    int ret = -1;

     virCommandPreserveFD(cmd, newfd1);
     virCommandTransferFD(cmd, newfd3);
@@ -188,21 +191,23 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
-        return -1;
+        goto cleanup;
     }

     if (fcntl(newfd1, F_GETFL) < 0 ||
         fcntl(newfd2, F_GETFL) < 0 ||
         fcntl(newfd3, F_GETFL) >= 0) {
         puts("fds in wrong state");
-        return -1;
+        goto cleanup;
     }

+    ret = checkoutput("test3");
+
+cleanup:
     virCommandFree(cmd);
     VIR_FORCE_CLOSE(newfd1);
     VIR_FORCE_CLOSE(newfd2);
-
-    return checkoutput("test3");
+    return ret;
 }


@@ -213,8 +218,12 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) {
  */
 static int test4(const void *unused ATTRIBUTE_UNUSED) {
     virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
-    pid_t pid;
     char *pidfile = virFilePid(abs_builddir, "commandhelper");
+    pid_t pid;
+    int ret = -1;
+
+    if (!pidfile)
+        goto cleanup;

     virCommandSetPidFile(cmd, pidfile);
     virCommandDaemonize(cmd);
@@ -222,21 +231,22 @@ static int test4(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
-        return -1;
+        goto cleanup;
     }

     if (virFileReadPid(abs_builddir, "commandhelper", &pid) != 0) {
         printf("cannot read pidfile\n");
-        return -1;
+        goto cleanup;
     }
     while (kill(pid, 0) != -1)
         usleep(100*1000);

-    virCommandFree(cmd);
+    ret = checkoutput("test4");

+cleanup:
+    virCommandFree(cmd);
     VIR_FREE(pidfile);
-
-    return checkoutput("test4");
+    return ret;
 }


@@ -252,6 +262,7 @@ static int test5(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -274,6 +285,7 @@ static int test6(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -297,6 +309,7 @@ static int test7(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -318,6 +331,7 @@ static int test8(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -343,6 +357,7 @@ static int test9(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -367,6 +382,7 @@ static int test10(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -389,6 +405,7 @@ static int test11(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -409,6 +426,7 @@ static int test12(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -435,22 +453,23 @@ static int test13(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
-        return -1;
+        goto cleanup;
     }
+    if (!outactual)
+        goto cleanup;

     virCommandFree(cmd);
+    cmd = NULL;

     if (!STREQ(outactual, outexpect)) {
         virtTestDifference(stderr, outactual, outexpect);
         goto cleanup;
     }

-    if (checkoutput("test13") < 0)
-        goto cleanup;
-
-    ret = 0;
+    ret = checkoutput("test13");

 cleanup:
+    virCommandFree(cmd);
     VIR_FREE(outactual);
     return ret;
 }
@@ -478,10 +497,13 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
-        return -1;
+        goto cleanup;
     }
+    if (!outactual || !erractual)
+        goto cleanup;

     virCommandFree(cmd);
+    cmd = NULL;

     if (!STREQ(outactual, outexpect)) {
         virtTestDifference(stderr, outactual, outexpect);
@@ -492,12 +514,10 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) {
         goto cleanup;
     }

-    if (checkoutput("test14") < 0)
-        goto cleanup;
-
-    ret = 0;
+    ret = checkoutput("test14");

 cleanup:
+    virCommandFree(cmd);
     VIR_FREE(outactual);
     VIR_FREE(erractual);
     return ret;
@@ -516,6 +536,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) {
     if (virCommandRun(cmd, NULL) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
+        virCommandFree(cmd);
         return -1;
     }

@@ -540,7 +561,7 @@ static int test16(const void *unused ATTRIBUTE_UNUSED) {
     if ((outactual = virCommandToString(cmd)) == NULL) {
         virErrorPtr err = virGetLastError();
         printf("Cannot convert to string: %s\n", err->message);
-        return -1;
+        goto cleanup;
     }
     if ((fd = open(abs_builddir "/commandhelper.log",
                    O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0) {
@@ -553,18 +574,15 @@ static int test16(const void *unused ATTRIBUTE_UNUSED) {
         goto cleanup;
     }

-    virCommandFree(cmd);
-
-    if (checkoutput("test16") < 0)
-        goto cleanup;
-
     if (!STREQ(outactual, outexpect)) {
         virtTestDifference(stderr, outactual, outexpect);
         goto cleanup;
     }
-    ret = 0;
+
+    ret = checkoutput("test16");

 cleanup:
+    virCommandFree(cmd);
     VIR_FORCE_CLOSE(fd);
     VIR_FREE(outactual);
     return ret;
-- 
1.7.3.2

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list