[libvirt] [PATCH v4 21/23] virSecuritySELinuxRestoreFileLabel: Adjust code pattern

Michal Privoznik mprivozn at redhat.com
Mon Sep 10 09:36:22 UTC 2018


Firstly, the following code pattern is harder to follow:

  if (func() < 0) {
      error();
  } else {
      /* success */
  }

We should put 'goto cleanup' into the error branch and move the
else branch one level up.
Secondly, 'rc' should really be named 'ret' because it holds
return value of the function. Not some intermediate value.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/security/security_selinux.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 35f18e1738..72d12c9df1 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1291,9 +1291,9 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
 {
     struct stat buf;
     security_context_t fcon = NULL;
-    int rc = -1;
     char *newpath = NULL;
     char ebuf[1024];
+    int ret = -1;
 
     /* Some paths are auto-generated, so let's be safe here and do
      * nothing if nothing is needed.
@@ -1320,15 +1320,18 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
          * which makes this an expected non error
          */
         VIR_WARN("cannot lookup default selinux label for %s", newpath);
-        rc = 0;
-    } else {
-        rc = virSecuritySELinuxSetFilecon(mgr, newpath, fcon);
+        ret = 0;
+        goto cleanup;
     }
 
+    if (virSecuritySELinuxSetFilecon(mgr, newpath, fcon) < 0)
+        goto cleanup;
+
+    ret = 0;
  cleanup:
     freecon(fcon);
     VIR_FREE(newpath);
-    return rc;
+    return ret;
 }
 
 
-- 
2.16.4




More information about the libvir-list mailing list