[libvirt PATCH 0/5] tests: commandtest: use g_auto more

Ján Tomko (5): tests: commandtest: remove unused 'prefix' parameter tests: commandtest: use g_autofree tests: commandtest: use g_autoptr for virCommand tests: commandtest: use VIR_AUTOCLOSE tests: commandtest: drop unnecessary labels tests/commandtest.c | 327 ++++++++++++++------------------------------ 1 file changed, 105 insertions(+), 222 deletions(-) -- 2.26.2

The 'checkoutput' function does have a parameter for a possible prefix, but it is now unused. Introduced-by: 241ac07124a3172dc38198d777eb3f04846f6c52 Used-by: 62f263a73e8202f2af7309d10000a3b9a66e6b57 Unused-since: 2dfacbffea47743fff942b6401f5f36b3ae4655a Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/commandtest.c | 52 ++++++++++++++++++--------------------------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index 9bef825239..a18100a705 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -55,8 +55,7 @@ main(void) /* Some UNIX lack it in headers & it doesn't hurt to redeclare */ extern char **environ; -static int checkoutput(const char *testname, - char *prefix) +static int checkoutput(const char *testname) { int ret = -1; char *expectname = NULL; @@ -77,15 +76,6 @@ static int checkoutput(const char *testname, goto cleanup; } - if (prefix) { - char *tmp = NULL; - - tmp = g_strdup_printf("%s%s", prefix, expectlog); - - VIR_FREE(expectlog); - expectlog = tmp; - } - if (STRNEQ(expectlog, actuallog)) { virTestDifference(stderr, expectlog, actuallog); goto cleanup; @@ -172,7 +162,7 @@ static int test2(const void *unused G_GNUC_UNUSED) return -1; } - if ((ret = checkoutput("test2", NULL)) != 0) { + if ((ret = checkoutput("test2")) != 0) { virCommandFree(cmd); return ret; } @@ -185,7 +175,7 @@ static int test2(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test2", NULL); + return checkoutput("test2"); } /* @@ -237,7 +227,7 @@ static int test3(const void *unused G_GNUC_UNUSED) } } - ret = checkoutput("test3", NULL); + ret = checkoutput("test3"); cleanup: virCommandFree(cmd); @@ -279,7 +269,7 @@ static int test4(const void *unused G_GNUC_UNUSED) while (kill(pid, 0) != -1) g_usleep(100*1000); - ret = checkoutput("test4", NULL); + ret = checkoutput("test4"); cleanup: virCommandFree(cmd); @@ -308,7 +298,7 @@ static int test5(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test5", NULL); + return checkoutput("test5"); } @@ -331,7 +321,7 @@ static int test6(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test6", NULL); + return checkoutput("test6"); } @@ -355,7 +345,7 @@ static int test7(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test7", NULL); + return checkoutput("test7"); } /* @@ -379,7 +369,7 @@ static int test8(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test8", NULL); + return checkoutput("test8"); } @@ -415,7 +405,7 @@ static int test9(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test9", NULL); + return checkoutput("test9"); } @@ -440,7 +430,7 @@ static int test10(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test10", NULL); + return checkoutput("test10"); } /* @@ -463,7 +453,7 @@ static int test11(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test11", NULL); + return checkoutput("test11"); } /* @@ -484,7 +474,7 @@ static int test12(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); - return checkoutput("test12", NULL); + return checkoutput("test12"); } /* @@ -518,7 +508,7 @@ static int test13(const void *unused G_GNUC_UNUSED) goto cleanup; } - ret = checkoutput("test13", NULL); + ret = checkoutput("test13"); cleanup: virCommandFree(cmd); @@ -588,7 +578,7 @@ static int test14(const void *unused G_GNUC_UNUSED) goto cleanup; } - ret = checkoutput("test14", NULL); + ret = checkoutput("test14"); cleanup: virCommandFree(cmd); @@ -618,7 +608,7 @@ static int test15(const void *unused G_GNUC_UNUSED) goto cleanup; } - ret = checkoutput("test15", NULL); + ret = checkoutput("test15"); cleanup: VIR_FREE(cwd); @@ -663,7 +653,7 @@ static int test16(const void *unused G_GNUC_UNUSED) goto cleanup; } - ret = checkoutput("test16", NULL); + ret = checkoutput("test16"); cleanup: virCommandFree(cmd); @@ -836,7 +826,7 @@ static int test20(const void *unused G_GNUC_UNUSED) goto cleanup; } - ret = checkoutput("test20", NULL); + ret = checkoutput("test20"); cleanup: virCommandFree(cmd); VIR_FREE(buf); @@ -894,7 +884,7 @@ static int test21(const void *unused G_GNUC_UNUSED) goto cleanup; } - ret = checkoutput("test21", NULL); + ret = checkoutput("test21"); cleanup: VIR_FREE(outbuf); VIR_FREE(errbuf); @@ -1136,7 +1126,7 @@ static int test26(const void *unused G_GNUC_UNUSED) goto cleanup; } - ret = checkoutput("test26", NULL); + ret = checkoutput("test26"); cleanup: virCommandFree(cmd); @@ -1234,7 +1224,7 @@ static int test27(const void *unused G_GNUC_UNUSED) goto cleanup; } - if (checkoutput("test27", NULL) < 0) + if (checkoutput("test27") < 0) goto cleanup; ret = 0; -- 2.26.2

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/commandtest.c | 76 ++++++++++++++++----------------------------- 1 file changed, 26 insertions(+), 50 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index a18100a705..7c6c3ec75d 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -58,10 +58,10 @@ extern char **environ; static int checkoutput(const char *testname) { int ret = -1; - char *expectname = NULL; - char *expectlog = NULL; - char *actualname = NULL; - char *actuallog = NULL; + g_autofree char *expectname = NULL; + g_autofree char *expectlog = NULL; + g_autofree char *actualname = NULL; + g_autofree char *actuallog = NULL; expectname = g_strdup_printf("%s/commanddata/%s.log", abs_srcdir, testname); actualname = g_strdup_printf("%s/commandhelper.log", abs_builddir); @@ -86,10 +86,6 @@ static int checkoutput(const char *testname) cleanup: if (actualname) unlink(actualname); - VIR_FREE(actuallog); - VIR_FREE(actualname); - VIR_FREE(expectlog); - VIR_FREE(expectname); return ret; } @@ -247,7 +243,7 @@ static int test4(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNewArgList(abs_builddir "/commandhelper", "--check-daemonize", NULL); - char *pidfile = virPidFileBuildPath(abs_builddir, "commandhelper"); + g_autofree char *pidfile = virPidFileBuildPath(abs_builddir, "commandhelper"); pid_t pid; int ret = -1; @@ -275,7 +271,6 @@ static int test4(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); if (pidfile) unlink(pidfile); - VIR_FREE(pidfile); return ret; } @@ -484,7 +479,7 @@ static int test12(const void *unused G_GNUC_UNUSED) static int test13(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); - char *outactual = NULL; + g_autofree char *outactual = NULL; const char *outexpect = "BEGIN STDOUT\n" "Hello World\n" "END STDOUT\n"; @@ -512,7 +507,6 @@ static int test13(const void *unused G_GNUC_UNUSED) cleanup: virCommandFree(cmd); - VIR_FREE(outactual); return ret; } @@ -523,16 +517,16 @@ static int test13(const void *unused G_GNUC_UNUSED) static int test14(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); - char *outactual = NULL; + g_autofree char *outactual = NULL; const char *outexpect = "BEGIN STDOUT\n" "Hello World\n" "END STDOUT\n"; - char *erractual = NULL; + g_autofree char *erractual = NULL; const char *errexpect = "BEGIN STDERR\n" "Hello World\n" "END STDERR\n"; - char *jointactual = NULL; + g_autofree char *jointactual = NULL; const char *jointexpect = "BEGIN STDOUT\n" "BEGIN STDERR\n" "Hello World\n" @@ -582,9 +576,6 @@ static int test14(const void *unused G_GNUC_UNUSED) cleanup: virCommandFree(cmd); - VIR_FREE(outactual); - VIR_FREE(erractual); - VIR_FREE(jointactual); return ret; } @@ -596,7 +587,7 @@ static int test14(const void *unused G_GNUC_UNUSED) static int test15(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); - char *cwd = NULL; + g_autofree char *cwd = NULL; int ret = -1; cwd = g_strdup_printf("%s/commanddata", abs_srcdir); @@ -611,7 +602,6 @@ static int test15(const void *unused G_GNUC_UNUSED) ret = checkoutput("test15"); cleanup: - VIR_FREE(cwd); virCommandFree(cmd); return ret; @@ -623,7 +613,7 @@ static int test15(const void *unused G_GNUC_UNUSED) static int test16(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNew("true"); - char *outactual = NULL; + g_autofree char *outactual = NULL; const char *outexpect = "A=B C='D E' true F 'G H'"; int ret = -1; int fd = -1; @@ -658,7 +648,6 @@ static int test16(const void *unused G_GNUC_UNUSED) cleanup: virCommandFree(cmd); VIR_FORCE_CLOSE(fd); - VIR_FREE(outactual); return ret; } @@ -669,8 +658,8 @@ static int test17(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNew("true"); int ret = -1; - char *outbuf; - char *errbuf = NULL; + char *outbuf = NULL; + g_autofree char *errbuf = NULL; virCommandSetOutputBuffer(cmd, &outbuf); if (outbuf != NULL) { @@ -711,7 +700,6 @@ static int test17(const void *unused G_GNUC_UNUSED) cleanup: virCommandFree(cmd); VIR_FREE(outbuf); - VIR_FREE(errbuf); return ret; } @@ -721,7 +709,7 @@ static int test17(const void *unused G_GNUC_UNUSED) static int test18(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNewArgList("sleep", "100", NULL); - char *pidfile = virPidFileBuildPath(abs_builddir, "commandhelper"); + g_autofree char *pidfile = virPidFileBuildPath(abs_builddir, "commandhelper"); pid_t pid; int ret = -1; @@ -759,7 +747,6 @@ static int test18(const void *unused G_GNUC_UNUSED) virCommandFree(cmd); if (pidfile) unlink(pidfile); - VIR_FREE(pidfile); return ret; } @@ -807,7 +794,7 @@ static int test20(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNewArgList(abs_builddir "/commandhelper", "--close-stdin", NULL); - char *buf; + g_autofree char *buf = NULL; int ret = -1; struct sigaction sig_action; @@ -829,7 +816,6 @@ static int test20(const void *unused G_GNUC_UNUSED) ret = checkoutput("test20"); cleanup: virCommandFree(cmd); - VIR_FREE(buf); return ret; } @@ -850,7 +836,8 @@ static int test21(const void *unused G_GNUC_UNUSED) virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); int ret = -1; const char *wrbuf = "Hello world\n"; - char *outbuf = NULL, *errbuf = NULL; + g_autofree char *outbuf = NULL; + g_autofree char *errbuf = NULL; const char *outbufExpected = "BEGIN STDOUT\n" "Hello world\n" "END STDOUT\n"; @@ -886,8 +873,6 @@ static int test21(const void *unused G_GNUC_UNUSED) ret = checkoutput("test21"); cleanup: - VIR_FREE(outbuf); - VIR_FREE(errbuf); virCommandFree(cmd); return ret; } @@ -1006,7 +991,7 @@ static int test25(const void *unused G_GNUC_UNUSED) int rv = 0; ssize_t tries = 100; pid_t pid; - gid_t *groups = NULL; + g_autofree gid_t *groups = NULL; int ngroups; virCommandPtr cmd = virCommandNew("some/nonexistent/binary"); @@ -1068,7 +1053,6 @@ static int test25(const void *unused G_GNUC_UNUSED) cleanup: VIR_FORCE_CLOSE(pipeFD[0]); VIR_FORCE_CLOSE(pipeFD[1]); - VIR_FREE(groups); virCommandFree(cmd); return ret; } @@ -1080,7 +1064,7 @@ static int test25(const void *unused G_GNUC_UNUSED) static int test26(const void *unused G_GNUC_UNUSED) { virCommandPtr cmd = virCommandNew("true"); - char *outactual = NULL; + g_autofree char *outactual = NULL; const char *outexpect = "A=B \\\n" "C='D E' \\\n" @@ -1131,7 +1115,6 @@ static int test26(const void *unused G_GNUC_UNUSED) cleanup: virCommandFree(cmd); VIR_FORCE_CLOSE(fd); - VIR_FREE(outactual); return ret; } @@ -1142,16 +1125,16 @@ static int test27(const void *unused G_GNUC_UNUSED) int pipe2[2]; int ret = -1; size_t buflen = 1024 * 128; - char *buffer0 = NULL; - char *buffer1 = NULL; - char *buffer2 = NULL; - char *outactual = NULL; - char *erractual = NULL; - char *outexpect = NULL; + g_autofree char *buffer0 = NULL; + g_autofree char *buffer1 = NULL; + g_autofree char *buffer2 = NULL; + g_autofree char *outactual = NULL; + g_autofree char *erractual = NULL; + g_autofree char *outexpect = NULL; # define TEST27_OUTEXPECT_TEMP "BEGIN STDOUT\n" \ "%s%s%s" \ "END STDOUT\n" - char *errexpect = NULL; + g_autofree char *errexpect = NULL; # define TEST27_ERREXPECT_TEMP "BEGIN STDERR\n" \ "%s%s%s" \ "END STDERR\n" @@ -1235,13 +1218,6 @@ static int test27(const void *unused G_GNUC_UNUSED) VIR_FORCE_CLOSE(pipe2[0]); VIR_FORCE_CLOSE(pipe1[1]); VIR_FORCE_CLOSE(pipe2[1]); - VIR_FREE(buffer0); - VIR_FREE(buffer1); - VIR_FREE(buffer2); - VIR_FREE(outactual); - VIR_FREE(erractual); - VIR_FREE(outexpect); - VIR_FREE(errexpect); return ret; } -- 2.26.2

Except for a few cases where freeing it explicitly seems to be done on purpose. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/commandtest.c | 94 ++++++++++++--------------------------------- 1 file changed, 25 insertions(+), 69 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index 7c6c3ec75d..a54e467b2b 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -96,7 +96,7 @@ static int checkoutput(const char *testname) */ static int test0(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd; + g_autoptr(virCommand) cmd = NULL; int ret = -1; cmd = virCommandNew(abs_builddir "/commandhelper-doesnotexist"); @@ -110,7 +110,6 @@ static int test0(const void *unused G_GNUC_UNUSED) ret = 0; cleanup: - virCommandFree(cmd); return ret; } @@ -121,7 +120,7 @@ static int test0(const void *unused G_GNUC_UNUSED) */ static int test1(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd; + g_autoptr(virCommand) cmd = NULL; int ret = -1; int status; @@ -139,7 +138,6 @@ static int test1(const void *unused G_GNUC_UNUSED) ret = 0; cleanup: - virCommandFree(cmd); return ret; } @@ -149,28 +147,22 @@ static int test1(const void *unused G_GNUC_UNUSED) */ static int test2(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); int ret; if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - if ((ret = checkoutput("test2")) != 0) { - virCommandFree(cmd); + if ((ret = checkoutput("test2")) != 0) return ret; - } if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test2"); } @@ -180,7 +172,7 @@ static int test2(const void *unused G_GNUC_UNUSED) */ static int test3(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); int newfd1 = dup(STDERR_FILENO); int newfd2 = dup(STDERR_FILENO); int newfd3 = dup(STDERR_FILENO); @@ -226,7 +218,6 @@ static int test3(const void *unused G_GNUC_UNUSED) ret = checkoutput("test3"); cleanup: - virCommandFree(cmd); /* coverity[double_close] */ VIR_FORCE_CLOSE(newfd1); VIR_FORCE_CLOSE(newfd2); @@ -241,8 +232,8 @@ static int test3(const void *unused G_GNUC_UNUSED) */ static int test4(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNewArgList(abs_builddir "/commandhelper", - "--check-daemonize", NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList(abs_builddir "/commandhelper", + "--check-daemonize", NULL); g_autofree char *pidfile = virPidFileBuildPath(abs_builddir, "commandhelper"); pid_t pid; int ret = -1; @@ -268,7 +259,6 @@ static int test4(const void *unused G_GNUC_UNUSED) ret = checkoutput("test4"); cleanup: - virCommandFree(cmd); if (pidfile) unlink(pidfile); return ret; @@ -281,18 +271,15 @@ static int test4(const void *unused G_GNUC_UNUSED) */ static int test5(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); virCommandAddEnvPassCommon(cmd); if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test5"); } @@ -303,19 +290,16 @@ static int test5(const void *unused G_GNUC_UNUSED) */ static int test6(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); virCommandAddEnvPass(cmd, "DISPLAY"); virCommandAddEnvPass(cmd, "DOESNOTEXIST"); if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test6"); } @@ -326,7 +310,7 @@ static int test6(const void *unused G_GNUC_UNUSED) */ static int test7(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); virCommandAddEnvPassCommon(cmd); virCommandAddEnvPass(cmd, "DISPLAY"); @@ -334,12 +318,9 @@ static int test7(const void *unused G_GNUC_UNUSED) if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test7"); } @@ -349,7 +330,7 @@ static int test7(const void *unused G_GNUC_UNUSED) */ static int test8(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); virCommandAddEnvString(cmd, "USER=bogus"); virCommandAddEnvString(cmd, "LANG=C"); @@ -358,12 +339,9 @@ static int test8(const void *unused G_GNUC_UNUSED) if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test8"); } @@ -374,7 +352,7 @@ static int test8(const void *unused G_GNUC_UNUSED) */ static int test9(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); const char* const args[] = { "arg1", "arg2", NULL }; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; @@ -388,18 +366,14 @@ static int test9(const void *unused G_GNUC_UNUSED) if (virBufferUse(&buf)) { printf("Buffer not transferred\n"); - virCommandFree(cmd); return -1; } if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test9"); } @@ -410,7 +384,7 @@ static int test9(const void *unused G_GNUC_UNUSED) */ static int test10(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); const char *const args[] = { "-version", "-log=bar.log", NULL, }; @@ -419,12 +393,9 @@ static int test10(const void *unused G_GNUC_UNUSED) if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test10"); } @@ -438,16 +409,13 @@ static int test11(const void *unused G_GNUC_UNUSED) abs_builddir "/commandhelper", "-version", "-log=bar.log", NULL, }; - virCommandPtr cmd = virCommandNewArgs(args); + g_autoptr(virCommand) cmd = virCommandNewArgs(args); if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test11"); } @@ -457,18 +425,15 @@ static int test11(const void *unused G_GNUC_UNUSED) */ static int test12(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); virCommandSetInputBuffer(cmd, "Hello World\n"); if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - virCommandFree(cmd); return -1; } - virCommandFree(cmd); - return checkoutput("test12"); } @@ -586,7 +551,7 @@ static int test14(const void *unused G_GNUC_UNUSED) */ static int test15(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); g_autofree char *cwd = NULL; int ret = -1; @@ -602,7 +567,6 @@ static int test15(const void *unused G_GNUC_UNUSED) ret = checkoutput("test15"); cleanup: - virCommandFree(cmd); return ret; } @@ -612,7 +576,7 @@ static int test15(const void *unused G_GNUC_UNUSED) */ static int test16(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew("true"); + g_autoptr(virCommand) cmd = virCommandNew("true"); g_autofree char *outactual = NULL; const char *outexpect = "A=B C='D E' true F 'G H'"; int ret = -1; @@ -646,7 +610,6 @@ static int test16(const void *unused G_GNUC_UNUSED) ret = checkoutput("test16"); cleanup: - virCommandFree(cmd); VIR_FORCE_CLOSE(fd); return ret; } @@ -656,7 +619,7 @@ static int test16(const void *unused G_GNUC_UNUSED) */ static int test17(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew("true"); + g_autoptr(virCommand) cmd = virCommandNew("true"); int ret = -1; char *outbuf = NULL; g_autofree char *errbuf = NULL; @@ -698,7 +661,6 @@ static int test17(const void *unused G_GNUC_UNUSED) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(outbuf); return ret; } @@ -755,7 +717,7 @@ static int test18(const void *unused G_GNUC_UNUSED) */ static int test19(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNewArgList("sleep", "100", NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList("sleep", "100", NULL); pid_t pid; int ret = -1; @@ -782,7 +744,6 @@ static int test19(const void *unused G_GNUC_UNUSED) ret = 0; cleanup: - virCommandFree(cmd); return ret; } @@ -792,8 +753,8 @@ static int test19(const void *unused G_GNUC_UNUSED) */ static int test20(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNewArgList(abs_builddir "/commandhelper", - "--close-stdin", NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList(abs_builddir "/commandhelper", + "--close-stdin", NULL); g_autofree char *buf = NULL; int ret = -1; @@ -815,7 +776,6 @@ static int test20(const void *unused G_GNUC_UNUSED) ret = checkoutput("test20"); cleanup: - virCommandFree(cmd); return ret; } @@ -833,7 +793,7 @@ static const char *const newenv[] = { static int test21(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); int ret = -1; const char *wrbuf = "Hello world\n"; g_autofree char *outbuf = NULL; @@ -873,7 +833,6 @@ static int test21(const void *unused G_GNUC_UNUSED) ret = checkoutput("test21"); cleanup: - virCommandFree(cmd); return ret; } @@ -993,7 +952,7 @@ static int test25(const void *unused G_GNUC_UNUSED) pid_t pid; g_autofree gid_t *groups = NULL; int ngroups; - virCommandPtr cmd = virCommandNew("some/nonexistent/binary"); + g_autoptr(virCommand) cmd = virCommandNew("some/nonexistent/binary"); if (virPipeQuiet(pipeFD) < 0) { fprintf(stderr, "Unable to create pipe\n"); @@ -1053,7 +1012,6 @@ static int test25(const void *unused G_GNUC_UNUSED) cleanup: VIR_FORCE_CLOSE(pipeFD[0]); VIR_FORCE_CLOSE(pipeFD[1]); - virCommandFree(cmd); return ret; } @@ -1063,7 +1021,7 @@ static int test25(const void *unused G_GNUC_UNUSED) */ static int test26(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew("true"); + g_autoptr(virCommand) cmd = virCommandNew("true"); g_autofree char *outactual = NULL; const char *outexpect = "A=B \\\n" @@ -1113,14 +1071,13 @@ static int test26(const void *unused G_GNUC_UNUSED) ret = checkoutput("test26"); cleanup: - virCommandFree(cmd); VIR_FORCE_CLOSE(fd); return ret; } static int test27(const void *unused G_GNUC_UNUSED) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); int pipe1[2]; int pipe2[2]; int ret = -1; @@ -1213,7 +1170,6 @@ static int test27(const void *unused G_GNUC_UNUSED) ret = 0; cleanup: - virCommandFree(cmd); VIR_FORCE_CLOSE(pipe1[0]); VIR_FORCE_CLOSE(pipe2[0]); VIR_FORCE_CLOSE(pipe1[1]); -- 2.26.2

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/commandtest.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index a54e467b2b..b0bf6f379a 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -173,8 +173,8 @@ static int test2(const void *unused G_GNUC_UNUSED) static int test3(const void *unused G_GNUC_UNUSED) { g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); - int newfd1 = dup(STDERR_FILENO); - int newfd2 = dup(STDERR_FILENO); + VIR_AUTOCLOSE newfd1 = dup(STDERR_FILENO); + VIR_AUTOCLOSE newfd2 = dup(STDERR_FILENO); int newfd3 = dup(STDERR_FILENO); struct stat before, after; int ret = -1; @@ -218,9 +218,6 @@ static int test3(const void *unused G_GNUC_UNUSED) ret = checkoutput("test3"); cleanup: - /* coverity[double_close] */ - VIR_FORCE_CLOSE(newfd1); - VIR_FORCE_CLOSE(newfd2); return ret; } @@ -580,7 +577,7 @@ static int test16(const void *unused G_GNUC_UNUSED) g_autofree char *outactual = NULL; const char *outexpect = "A=B C='D E' true F 'G H'"; int ret = -1; - int fd = -1; + VIR_AUTOCLOSE fd = -1; virCommandAddEnvPair(cmd, "A", "B"); virCommandAddEnvPair(cmd, "C", "D E"); @@ -610,7 +607,6 @@ static int test16(const void *unused G_GNUC_UNUSED) ret = checkoutput("test16"); cleanup: - VIR_FORCE_CLOSE(fd); return ret; } @@ -1039,7 +1035,7 @@ static int test26(const void *unused G_GNUC_UNUSED) "wallop"; int ret = -1; - int fd = -1; + VIR_AUTOCLOSE fd = -1; virCommandAddEnvPair(cmd, "A", "B"); virCommandAddEnvPair(cmd, "C", "D E"); @@ -1071,7 +1067,6 @@ static int test26(const void *unused G_GNUC_UNUSED) ret = checkoutput("test26"); cleanup: - VIR_FORCE_CLOSE(fd); return ret; } -- 2.26.2

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/commandtest.c | 106 +++++++++++++++----------------------------- 1 file changed, 36 insertions(+), 70 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index b0bf6f379a..42225a8ef2 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -97,20 +97,16 @@ static int checkoutput(const char *testname) static int test0(const void *unused G_GNUC_UNUSED) { g_autoptr(virCommand) cmd = NULL; - int ret = -1; cmd = virCommandNew(abs_builddir "/commandhelper-doesnotexist"); if (virCommandRun(cmd, NULL) == 0) - goto cleanup; + return -1; if (virGetLastErrorCode() == VIR_ERR_OK) - goto cleanup; + return -1; virResetLastError(); - ret = 0; - - cleanup: - return ret; + return 0; } /* @@ -121,24 +117,21 @@ static int test0(const void *unused G_GNUC_UNUSED) static int test1(const void *unused G_GNUC_UNUSED) { g_autoptr(virCommand) cmd = NULL; - int ret = -1; int status; cmd = virCommandNew(abs_builddir "/commandhelper-doesnotexist"); if (virCommandRun(cmd, &status) < 0) - goto cleanup; + return -1; if (status != EXIT_ENOENT) - goto cleanup; + return -1; virCommandRawStatus(cmd); if (virCommandRun(cmd, &status) < 0) - goto cleanup; + return -1; if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_ENOENT) - goto cleanup; - ret = 0; + return -1; - cleanup: - return ret; + return 0; } /* @@ -177,11 +170,10 @@ static int test3(const void *unused G_GNUC_UNUSED) VIR_AUTOCLOSE newfd2 = dup(STDERR_FILENO); int newfd3 = dup(STDERR_FILENO); struct stat before, after; - int ret = -1; if (fstat(newfd3, &before) < 0) { perror("fstat"); - goto cleanup; + return -1; } virCommandPassFD(cmd, newfd1, 0); virCommandPassFD(cmd, newfd3, @@ -189,13 +181,13 @@ static int test3(const void *unused G_GNUC_UNUSED) if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - goto cleanup; + return -1; } if (fcntl(newfd1, F_GETFL) < 0 || fcntl(newfd2, F_GETFL) < 0) { puts("fds 1/2 were not open"); - goto cleanup; + return -1; } /* We expect newfd3 to be closed, but the @@ -211,14 +203,11 @@ static int test3(const void *unused G_GNUC_UNUSED) before.st_dev == after.st_dev && before.st_mode == after.st_mode) { puts("fd 3 should not be open"); - goto cleanup; + return -1; } } - ret = checkoutput("test3"); - - cleanup: - return ret; + return checkoutput("test3"); } @@ -550,7 +539,6 @@ static int test15(const void *unused G_GNUC_UNUSED) { g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); g_autofree char *cwd = NULL; - int ret = -1; cwd = g_strdup_printf("%s/commanddata", abs_srcdir); virCommandSetWorkingDirectory(cmd, cwd); @@ -558,14 +546,10 @@ static int test15(const void *unused G_GNUC_UNUSED) if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - goto cleanup; + return -1; } - ret = checkoutput("test15"); - - cleanup: - - return ret; + return checkoutput("test15"); } /* @@ -576,7 +560,6 @@ static int test16(const void *unused G_GNUC_UNUSED) g_autoptr(virCommand) cmd = virCommandNew("true"); g_autofree char *outactual = NULL; const char *outexpect = "A=B C='D E' true F 'G H'"; - int ret = -1; VIR_AUTOCLOSE fd = -1; virCommandAddEnvPair(cmd, "A", "B"); @@ -586,28 +569,25 @@ static int test16(const void *unused G_GNUC_UNUSED) if ((outactual = virCommandToString(cmd, false)) == NULL) { printf("Cannot convert to string: %s\n", virGetLastErrorMessage()); - goto cleanup; + return -1; } if ((fd = open(abs_builddir "/commandhelper.log", O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0) { printf("Cannot open log file: %s\n", g_strerror(errno)); - goto cleanup; + return -1; } virCommandWriteArgLog(cmd, fd); if (VIR_CLOSE(fd) < 0) { printf("Cannot close log file: %s\n", g_strerror(errno)); - goto cleanup; + return -1; } if (STRNEQ(outactual, outexpect)) { virTestDifference(stderr, outexpect, outactual); - goto cleanup; + return -1; } - ret = checkoutput("test16"); - - cleanup: - return ret; + return checkoutput("test16"); } /* @@ -715,32 +695,28 @@ static int test19(const void *unused G_GNUC_UNUSED) { g_autoptr(virCommand) cmd = virCommandNewArgList("sleep", "100", NULL); pid_t pid; - int ret = -1; alarm(5); if (virCommandRunAsync(cmd, &pid) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - goto cleanup; + return -1; } if (kill(pid, 0) != 0) { printf("Child should still be running"); - goto cleanup; + return -1; } virCommandAbort(cmd); if (kill(pid, 0) == 0) { printf("Child should be aborted"); - goto cleanup; + return -1; } alarm(0); - ret = 0; - - cleanup: - return ret; + return 0; } /* @@ -752,7 +728,6 @@ static int test20(const void *unused G_GNUC_UNUSED) g_autoptr(virCommand) cmd = virCommandNewArgList(abs_builddir "/commandhelper", "--close-stdin", NULL); g_autofree char *buf = NULL; - int ret = -1; struct sigaction sig_action; @@ -767,12 +742,10 @@ static int test20(const void *unused G_GNUC_UNUSED) if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - goto cleanup; + return -1; } - ret = checkoutput("test20"); - cleanup: - return ret; + return checkoutput("test20"); } static const char *const newenv[] = { @@ -790,7 +763,6 @@ static const char *const newenv[] = { static int test21(const void *unused G_GNUC_UNUSED) { g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper"); - int ret = -1; const char *wrbuf = "Hello world\n"; g_autofree char *outbuf = NULL; g_autofree char *errbuf = NULL; @@ -808,28 +780,26 @@ static int test21(const void *unused G_GNUC_UNUSED) if (virCommandRunAsync(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); - goto cleanup; + return -1; } if (virCommandWait(cmd, NULL) < 0) - goto cleanup; + return -1; if (virTestGetVerbose()) printf("STDOUT:%s\nSTDERR:%s\n", NULLSTR(outbuf), NULLSTR(errbuf)); if (STRNEQ_NULLABLE(outbuf, outbufExpected)) { virTestDifference(stderr, outbufExpected, outbuf); - goto cleanup; + return -1; } if (STRNEQ_NULLABLE(errbuf, errbufExpected)) { virTestDifference(stderr, errbufExpected, errbuf); - goto cleanup; + return -1; } - ret = checkoutput("test21"); - cleanup: - return ret; + return checkoutput("test21"); } static int @@ -1034,7 +1004,6 @@ static int test26(const void *unused G_GNUC_UNUSED) "bang \\\n" "wallop"; - int ret = -1; VIR_AUTOCLOSE fd = -1; virCommandAddEnvPair(cmd, "A", "B"); @@ -1046,28 +1015,25 @@ static int test26(const void *unused G_GNUC_UNUSED) if ((outactual = virCommandToString(cmd, true)) == NULL) { printf("Cannot convert to string: %s\n", virGetLastErrorMessage()); - goto cleanup; + return -1; } if ((fd = open(abs_builddir "/commandhelper.log", O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0) { printf("Cannot open log file: %s\n", g_strerror(errno)); - goto cleanup; + return -1; } virCommandWriteArgLog(cmd, fd); if (VIR_CLOSE(fd) < 0) { printf("Cannot close log file: %s\n", g_strerror(errno)); - goto cleanup; + return -1; } if (STRNEQ(outactual, outexpect)) { virTestDifference(stderr, outexpect, outactual); - goto cleanup; + return -1; } - ret = checkoutput("test26"); - - cleanup: - return ret; + return checkoutput("test26"); } static int test27(const void *unused G_GNUC_UNUSED) -- 2.26.2

On 7/28/20 7:40 PM, Ján Tomko wrote:
Ján Tomko (5): tests: commandtest: remove unused 'prefix' parameter tests: commandtest: use g_autofree tests: commandtest: use g_autoptr for virCommand tests: commandtest: use VIR_AUTOCLOSE tests: commandtest: drop unnecessary labels
tests/commandtest.c | 327 ++++++++++++++------------------------------ 1 file changed, 105 insertions(+), 222 deletions(-)
For the series: Reviewed-by: Laine Stump <laine@redhat.com>
participants (2)
-
Ján Tomko
-
Laine Stump