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(a)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