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 | 35 +++++++++++---------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
index 09902e1c54..b62162fe9f 100644
--- a/tests/securityselinuxlabeltest.c
+++ b/tests/securityselinuxlabeltest.c
@@ -82,16 +82,12 @@ 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);
+    char *tmp = g_strdup_printf("%s/securityselinuxlabeldata%s", abs_builddir, *path);
+    g_free(*path);
     *path = tmp;
-    return 0;
 }
 
 static int
@@ -154,7 +150,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;
 
@@ -163,15 +159,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++) {
@@ -182,23 +177,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)
+        testSELinuxMungePath(&def->os.kernel);
+    if (def->os.initrd)
+        testSELinuxMungePath(&def->os.initrd);
 
- cleanup:
-    VIR_FREE(xmlfile);
     return def;
 }
 
-- 
2.34.1

On 1/28/22 19:41, Ján Tomko wrote:
> 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 | 35 +++++++++++---------------------
>  1 file changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
> index 09902e1c54..b62162fe9f 100644
> --- a/tests/securityselinuxlabeltest.c
> +++ b/tests/securityselinuxlabeltest.c
> @@ -82,16 +82,12 @@ 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);
> +    char *tmp = g_strdup_printf("%s/securityselinuxlabeldata%s", abs_builddir, *path);
> +    g_free(*path);
>      *path = tmp;
> -    return 0;

nitpick: please keep that empty line between variable declaration block
and code block.

>  }

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal