[libvirt PATCH 0/6] tests: refactor some functions (glib chronicles)
Ján Tomko (6): tests: esxutils: refactor testParseDatastorePath tests: esxutils: reduce variable scope in testEscapeDatastoreItem tests: esxutils: reduce variable scope in testConvertWindows1252ToUTF8 tests: reduce variable scope in testSELinuxCheckLabels tests: refactor testSELinuxLoadDef virnetsockettest: refactor checkProtocols tests/esxutilstest.c | 46 +++++++++----------------------- tests/securityselinuxlabeltest.c | 41 +++++++++------------------- tests/virnetsockettest.c | 26 +++++++----------- 3 files changed, 34 insertions(+), 79 deletions(-) -- 2.31.1
Reduce variable scope to match their lifetime, use g_auto and remove now pointless labels in favor of direct returns. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/esxutilstest.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c index 2e2020006e..bfb35fa356 100644 --- a/tests/esxutilstest.c +++ b/tests/esxutilstest.c @@ -35,21 +35,17 @@ static struct testPath paths[] = { static int testParseDatastorePath(const void *data G_GNUC_UNUSED) { - int result = 0; size_t i; - char *datastoreName = NULL; - char *directoryName = NULL; - char *directoryAndFileName = NULL; for (i = 0; i < G_N_ELEMENTS(paths); ++i) { - VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(directoryAndFileName); + g_autofree char *datastoreName = NULL; + g_autofree char *directoryName = NULL; + g_autofree char *directoryAndFileName = NULL; if (esxUtil_ParseDatastorePath (paths[i].datastorePath, &datastoreName, &directoryName, &directoryAndFileName) != paths[i].result) { - goto failure; + return -1; } if (paths[i].result < 0) @@ -57,32 +53,22 @@ testParseDatastorePath(const void *data G_GNUC_UNUSED) if (STRNEQ(paths[i].datastoreName, datastoreName)) { virTestDifference(stderr, paths[i].datastoreName, datastoreName); - goto failure; + return -1; } if (STRNEQ(paths[i].directoryName, directoryName)) { virTestDifference(stderr, paths[i].directoryName, directoryName); - goto failure; + return -1; } if (STRNEQ(paths[i].directoryAndFileName, directoryAndFileName)) { virTestDifference(stderr, paths[i].directoryAndFileName, directoryAndFileName); - goto failure; + return -1; } } - cleanup: - VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(directoryAndFileName); - - return result; - - failure: - result = -1; - - goto cleanup; + return 0; } -- 2.31.1
Also use g_auto. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/esxutilstest.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c index bfb35fa356..d537da3011 100644 --- a/tests/esxutilstest.c +++ b/tests/esxutilstest.c @@ -161,23 +161,19 @@ static int testEscapeDatastoreItem(const void *data G_GNUC_UNUSED) { size_t i; - char *escaped = NULL; for (i = 0; i < G_N_ELEMENTS(datastoreItems); ++i) { - VIR_FREE(escaped); + g_autofree char *escaped = NULL; escaped = esxUtil_EscapeDatastoreItem(datastoreItems[i].string); if (escaped == NULL) return -1; - if (STRNEQ(datastoreItems[i].escaped, escaped)) { - VIR_FREE(escaped); + if (STRNEQ(datastoreItems[i].escaped, escaped)) return -1; - } } - VIR_FREE(escaped); return 0; } -- 2.31.1
Also use g_auto. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/esxutilstest.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c index d537da3011..db49dcd6e0 100644 --- a/tests/esxutilstest.c +++ b/tests/esxutilstest.c @@ -197,10 +197,9 @@ static int testConvertWindows1252ToUTF8(const void *data G_GNUC_UNUSED) { size_t i; - char *utf8 = NULL; for (i = 0; i < G_N_ELEMENTS(windows1252ToUTF8); ++i) { - VIR_FREE(utf8); + g_autofree char *utf8 = NULL; utf8 = virVMXConvertToUTF8("Windows-1252", windows1252ToUTF8[i].windows1252); @@ -208,13 +207,10 @@ testConvertWindows1252ToUTF8(const void *data G_GNUC_UNUSED) if (utf8 == NULL) return -1; - if (STRNEQ(windows1252ToUTF8[i].utf8, utf8)) { - VIR_FREE(utf8); + if (STRNEQ(windows1252ToUTF8[i].utf8, utf8)) return -1; - } } - VIR_FREE(utf8); return 0; } -- 2.31.1
And use g_auto. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/securityselinuxlabeltest.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index f7dd3c866a..3d886fd827 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -246,10 +246,9 @@ static int testSELinuxCheckLabels(testSELinuxFile *files, size_t nfiles) { size_t i; - char *ctx; for (i = 0; i < nfiles; i++) { - ctx = NULL; + g_autofree char *ctx = NULL; if (getfilecon(files[i].file, &ctx) < 0) { if (errno == ENODATA) { /* nothing to do */ @@ -266,10 +265,8 @@ testSELinuxCheckLabels(testSELinuxFile *files, size_t nfiles) virReportError(VIR_ERR_INTERNAL_ERROR, "File %s context '%s' did not match expected '%s'", files[i].file, ctx, files[i].context); - VIR_FREE(ctx); return -1; } - VIR_FREE(ctx); } return 0; } -- 2.31.1
Since its introduction in commit 907a39e735d256b8428ed4c77009d1f713aea19b Add a test suite for validating SELinux labelling this function did not return NULL on OOM. Since we abort on OOM now, switch testSELinuxMungePath to void, return NULL explicitly on XML parsing failure and remove the (now pointless) cleanup label. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/securityselinuxlabeltest.c | 36 +++++++++++--------------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 3d886fd827..aab1561335 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -83,16 +83,11 @@ testUserXattrEnabled(void) return ret; } -static int +static void testSELinuxMungePath(char **path) { - char *tmp; - - tmp = g_strdup_printf("%s/securityselinuxlabeldata%s", abs_builddir, *path); - - VIR_FREE(*path); - *path = tmp; - return 0; + g_free(*path); + *path = g_strdup_printf("%s/securityselinuxlabeldata%s", abs_builddir, *path); } static int @@ -162,7 +157,7 @@ testSELinuxLoadFileList(const char *testname, static virDomainDef * testSELinuxLoadDef(const char *testname) { - char *xmlfile = NULL; + g_autofree char *xmlfile = NULL; virDomainDef *def = NULL; size_t i; @@ -171,15 +166,14 @@ testSELinuxLoadDef(const char *testname) if (!(def = virDomainDefParseFile(xmlfile, driver.xmlopt, NULL, 0))) - goto cleanup; + return NULL; for (i = 0; i < def->ndisks; i++) { if (def->disks[i]->src->type != VIR_STORAGE_TYPE_FILE && def->disks[i]->src->type != VIR_STORAGE_TYPE_BLOCK) continue; - if (testSELinuxMungePath(&def->disks[i]->src->path) < 0) - goto cleanup; + testSELinuxMungePath(&def->disks[i]->src->path); } for (i = 0; i < def->nserials; i++) { @@ -190,23 +184,17 @@ testSELinuxLoadDef(const char *testname) continue; if (def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX) { - if (testSELinuxMungePath(&def->serials[i]->source->data.nix.path) < 0) - goto cleanup; + testSELinuxMungePath(&def->serials[i]->source->data.nix.path); } else { - if (testSELinuxMungePath(&def->serials[i]->source->data.file.path) < 0) - goto cleanup; + testSELinuxMungePath(&def->serials[i]->source->data.file.path); } } - if (def->os.kernel && - testSELinuxMungePath(&def->os.kernel) < 0) - goto cleanup; - if (def->os.initrd && - testSELinuxMungePath(&def->os.initrd) < 0) - goto cleanup; + if (def->os.kernel < 0) + testSELinuxMungePath(&def->os.kernel); + if (def->os.initrd < 0) + testSELinuxMungePath(&def->os.initrd); - cleanup: - VIR_FREE(xmlfile); return def; } -- 2.31.1
Reduce variable scope, use g_auto and remove pointless labels. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virnetsockettest.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 8059c6cbb0..1de3771ad4 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -47,28 +47,29 @@ checkProtocols(bool *hasIPv4, bool *hasIPv6, { struct sockaddr_in in4; struct sockaddr_in6 in6; - int s4 = -1, s6 = -1; size_t i; - int ret = -1; *freePort = 0; if (virNetSocketCheckProtocols(hasIPv4, hasIPv6) < 0) return -1; for (i = 0; i < 50; i++) { + VIR_AUTOCLOSE s4 = -1; + VIR_AUTOCLOSE s6 = -1; + if (*hasIPv4) { if ((s4 = socket(AF_INET, SOCK_STREAM, 0)) < 0) - goto cleanup; + return -1; } if (*hasIPv6) { int only = 1; if ((s6 = socket(AF_INET6, SOCK_STREAM, 0)) < 0) - goto cleanup; + return -1; if (setsockopt(s6, IPPROTO_IPV6, IPV6_V6ONLY, &only, sizeof(only)) < 0) - goto cleanup; + return -1; } memset(&in4, 0, sizeof(in4)); @@ -84,22 +85,18 @@ checkProtocols(bool *hasIPv4, bool *hasIPv6, if (*hasIPv4) { if (bind(s4, (struct sockaddr *)&in4, sizeof(in4)) < 0) { if (errno == EADDRINUSE) { - VIR_FORCE_CLOSE(s4); - VIR_FORCE_CLOSE(s6); continue; } - goto cleanup; + return -1; } } if (*hasIPv6) { if (bind(s6, (struct sockaddr *)&in6, sizeof(in6)) < 0) { if (errno == EADDRINUSE) { - VIR_FORCE_CLOSE(s4); - VIR_FORCE_CLOSE(s6); continue; } - goto cleanup; + return -1; } } @@ -109,12 +106,7 @@ checkProtocols(bool *hasIPv4, bool *hasIPv6, VIR_DEBUG("Choose port %d", *freePort); - ret = 0; - - cleanup: - VIR_FORCE_CLOSE(s4); - VIR_FORCE_CLOSE(s6); - return ret; + return 0; } struct testClientData { -- 2.31.1
On Sat, Sep 04, 2021 at 09:34:46PM +0200, Ján Tomko wrote:
Ján Tomko (6): tests: esxutils: refactor testParseDatastorePath tests: esxutils: reduce variable scope in testEscapeDatastoreItem tests: esxutils: reduce variable scope in testConvertWindows1252ToUTF8 tests: reduce variable scope in testSELinuxCheckLabels tests: refactor testSELinuxLoadDef virnetsockettest: refactor checkProtocols
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Ján Tomko -
Pavel Hrdina