[PATCH v2 2/4] security: replace uses of label and VIR_FREE by g_autofree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Moving towards full adoption of GLib APIs in the AppArmor code.

Signed-off-by: Georgia Garcia <georgia.garcia@xxxxxxxxxxxxx>
---
 src/security/security_apparmor.c |  41 ++++---------
 src/security/virt-aa-helper.c    | 100 ++++++++++---------------------
 2 files changed, 45 insertions(+), 96 deletions(-)

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 7092724563..9e578b2526 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -115,37 +115,28 @@ profile_loaded(const char *str)
 static int
 profile_status_file(const char *str)
 {
-    char *profile = NULL;
-    char *content = NULL;
-    char *tmp = NULL;
-    int rc = -1;
+    g_autofree char *profile = NULL;
+    g_autofree char *content = NULL;
+    g_autofree char *tmp = NULL;
     int len;
 
     profile = g_strdup_printf("%s/%s", APPARMOR_DIR "/libvirt", str);
 
     if (!virFileExists(profile))
-        goto failed;
+        return -1;
 
     if ((len = virFileReadAll(profile, MAX_FILE_LEN, &content)) < 0) {
         virReportSystemError(errno,
                              _("Failed to read \'%1$s\'"), profile);
-        goto failed;
+        return -1;
     }
 
     /* create string that is ' <str> flags=(complain)\0' */
     tmp = g_strdup_printf(" %s flags=(complain)", str);
 
     if (strstr(content, tmp) != NULL)
-        rc = 0;
-    else
-        rc = 1;
-
- failed:
-    VIR_FREE(tmp);
-    VIR_FREE(profile);
-    VIR_FREE(content);
-
-    return rc;
+        return 0;
+    return 1;
 }
 
 /*
@@ -218,7 +209,7 @@ static int
 use_apparmor(void)
 {
     int rc = -1;
-    char *libvirt_daemon = NULL;
+    g_autofree char *libvirt_daemon = NULL;
 
     if (virFileResolveLink("/proc/self/exe", &libvirt_daemon) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -232,7 +223,7 @@ use_apparmor(void)
         return 1;
 
     if (access(APPARMOR_PROFILES_PATH, R_OK) != 0)
-        goto cleanup;
+        return rc;
 
     /* First check profile status using full binary path. If that fails
      * check using profile name.
@@ -245,8 +236,6 @@ use_apparmor(void)
             rc = -1;
     }
 
- cleanup:
-    VIR_FREE(libvirt_daemon);
     return rc;
 }
 
@@ -948,7 +937,7 @@ AppArmorSetChardevLabel(virSecurityManager *mgr,
                         virDomainChrSourceDef *dev_source,
                         bool chardevStdioLogd G_GNUC_UNUSED)
 {
-    char *in = NULL, *out = NULL;
+    g_autofree char *in = NULL, *out = NULL;
     int ret = -1;
     virSecurityLabelDef *secdef;
 
@@ -969,11 +958,11 @@ AppArmorSetChardevLabel(virSecurityManager *mgr,
         out = g_strdup_printf("%s.out", dev_source->data.file.path);
         if (virFileExists(in)) {
             if (reload_profile(mgr, def, in, true) < 0)
-                goto done;
+                return ret;
         }
         if (virFileExists(out)) {
             if (reload_profile(mgr, def, out, true) < 0)
-                goto done;
+                return ret;
         }
         ret = reload_profile(mgr, def, dev_source->data.file.path, true);
         break;
@@ -993,9 +982,6 @@ AppArmorSetChardevLabel(virSecurityManager *mgr,
         break;
     }
 
- done:
-    VIR_FREE(in);
-    VIR_FREE(out);
     return ret;
 }
 
@@ -1081,12 +1067,11 @@ AppArmorSetPathLabel(virSecurityManager *mgr,
                            bool allowSubtree)
 {
     int rc = -1;
-    char *full_path = NULL;
+    g_autofree char *full_path = NULL;
 
     if (allowSubtree) {
         full_path = g_strdup_printf("%s/{,**}", path);
         rc = reload_profile(mgr, def, full_path, true);
-        VIR_FREE(full_path);
     } else {
         rc = reload_profile(mgr, def, path, true);
     }
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 067a17f331..601f2d2581 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -146,9 +146,8 @@ vah_info(const char *str)
 static int
 parserCommand(const char *profile_name, const char cmd)
 {
-    int result = -1;
     char flag[3];
-    char *profile;
+    g_autofree char *profile = NULL;
     int status;
     int ret;
 
@@ -163,7 +162,7 @@ parserCommand(const char *profile_name, const char cmd)
 
     if (!virFileExists(profile)) {
         vah_error(NULL, 0, _("profile does not exist"));
-        goto cleanup;
+        return -1;
     } else {
         const char * const argv[] = {
             "/sbin/apparmor_parser", flag, profile, NULL
@@ -175,23 +174,18 @@ parserCommand(const char *profile_name, const char cmd)
             (WIFEXITED(status) && WEXITSTATUS(status) != 0)) {
             if (ret != 0) {
                 vah_error(NULL, 0, _("failed to run apparmor_parser"));
-                goto cleanup;
+                return -1;
             } else if (cmd == 'R' && WIFEXITED(status) &&
                        WEXITSTATUS(status) == 234) {
                 vah_warning(_("unable to unload already unloaded profile"));
             } else {
                 vah_error(NULL, 0, _("apparmor_parser exited with error"));
-                goto cleanup;
+                return -1;
             }
         }
     }
 
-    result = 0;
-
- cleanup:
-    VIR_FREE(profile);
-
-    return result;
+    return 0;
 }
 
 /*
@@ -201,18 +195,17 @@ static int
 update_include_file(const char *include_file, const char *included_files,
                     bool append)
 {
-    int rc = -1;
     int plen, flen = 0;
     int fd;
-    char *pcontent = NULL;
-    char *existing = NULL;
+    g_autofree char *pcontent = NULL;
+    g_autofree char *existing = NULL;
     const char *warning =
          "# DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT.\n";
 
     if (virFileExists(include_file)) {
         flen = virFileReadAll(include_file, MAX_FILE_LEN, &existing);
         if (flen < 0)
-            return rc;
+            return -1;
     }
 
     if (append && virFileExists(include_file))
@@ -223,38 +216,31 @@ update_include_file(const char *include_file, const char *included_files,
     plen = strlen(pcontent);
     if (plen > MAX_FILE_LEN) {
         vah_error(NULL, 0, _("invalid length for new profile"));
-        goto cleanup;
+        return -1;
     }
 
     /* only update the disk profile if it is different */
     if (flen > 0 && flen == plen && STREQLEN(existing, pcontent, plen)) {
-        rc = 0;
-        goto cleanup;
+        return 0;
     }
 
     /* write the file */
     if ((fd = open(include_file, O_CREAT | O_TRUNC | O_WRONLY, 0644)) == -1) {
         vah_error(NULL, 0, _("failed to create include file"));
-        goto cleanup;
+        return -1;
     }
 
     if (safewrite(fd, pcontent, plen) < 0) { /* don't write the '\0' */
         VIR_FORCE_CLOSE(fd);
         vah_error(NULL, 0, _("failed to write to profile"));
-        goto cleanup;
+        return -1;
     }
 
     if (VIR_CLOSE(fd) != 0) {
         vah_error(NULL, 0, _("failed to close or write to profile"));
-        goto cleanup;
+        return -1;
     }
-    rc = 0;
-
- cleanup:
-    VIR_FREE(pcontent);
-    VIR_FREE(existing);
-
-    return rc;
+    return 0;
 }
 
 /*
@@ -572,7 +558,7 @@ caps_mockup(vahControl * ctl, const char *xmlStr)
 {
     g_autoptr(xmlDoc) xml = NULL;
     g_autoptr(xmlXPathContext) ctxt = NULL;
-    char *arch;
+    g_autofree char *arch = NULL;
 
     if (!(xml = virXMLParse(NULL, xmlStr, _("(domain_definition)"),
                             "domain", &ctxt, NULL, false))) {
@@ -598,7 +584,6 @@ caps_mockup(vahControl * ctl, const char *xmlStr)
         ctl->arch = virArchFromHost();
     } else {
         ctl->arch = virArchFromString(arch);
-        VIR_FREE(arch);
     }
 
     return 0;
@@ -683,15 +668,15 @@ get_definition(vahControl * ctl, const char *xmlStr)
 static int
 vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive)
 {
-    char *tmp = NULL;
     int rc = -1;
     bool readonly = true;
     bool explicit_deny_rule = true;
     char *sub = NULL;
-    char *perms_new = NULL;
-    char *pathdir = NULL;
-    char *pathtmp = NULL;
-    char *pathreal = NULL;
+    g_autofree char *tmp = NULL;
+    g_autofree char *perms_new = NULL;
+    g_autofree char *pathdir = NULL;
+    g_autofree char *pathtmp = NULL;
+    g_autofree char *pathreal = NULL;
 
     if (path == NULL)
         return rc;
@@ -728,7 +713,7 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive
         if ((pathreal = realpath(pathdir, NULL)) == NULL) {
             vah_error(NULL, 0, pathdir);
             vah_error(NULL, 0, _("could not find realpath"));
-            goto cleanup;
+            return rc;
         }
         tmp = g_strdup_printf("%s%s", pathreal, pathtmp);
     }
@@ -752,7 +737,7 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive
             vah_error(NULL, 0, path);
             vah_error(NULL, 0, _("skipped restricted file"));
         }
-        goto cleanup;
+        return rc;
     }
 
     if (tmp[strlen(tmp) - 1] == '/')
@@ -769,13 +754,6 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive
         virBufferAsprintf(buf, "  \"%s/\" r,\n", tmp);
     }
 
- cleanup:
-    VIR_FREE(pathdir);
-    VIR_FREE(pathtmp);
-    VIR_FREE(pathreal);
-    VIR_FREE(perms_new);
-    VIR_FREE(tmp);
-
     return rc;
 }
 
@@ -791,36 +769,28 @@ vah_add_file_chardev(virBuffer *buf,
                      const char *perms,
                      const int type)
 {
-    char *pipe_in;
-    char *pipe_out;
-    int rc = -1;
+    g_autofree char *pipe_in = NULL;
+    g_autofree char *pipe_out = NULL;
 
     if (type == VIR_DOMAIN_CHR_TYPE_PIPE) {
         /* add the pipe input */
         pipe_in = g_strdup_printf("%s.in", path);
 
         if (vah_add_file(buf, pipe_in, perms) != 0)
-            goto clean_pipe_in;
+            return -1;
 
         /* add the pipe output */
         pipe_out = g_strdup_printf("%s.out", path);
 
         if (vah_add_file(buf, pipe_out, perms) != 0)
-            goto clean_pipe_out;
-
-        rc = 0;
-      clean_pipe_out:
-        VIR_FREE(pipe_out);
-      clean_pipe_in:
-        VIR_FREE(pipe_in);
+            return -1;
     } else {
         /* add the file */
         if (vah_add_file(buf, path, perms) != 0)
             return -1;
-        rc = 0;
     }
 
-    return rc;
+    return 0;
 }
 
 static int
@@ -1467,8 +1437,8 @@ main(int argc, char **argv)
     vahControl _ctl = { 0 };
     vahControl *ctl = &_ctl;
     int rc = -1;
-    char *profile = NULL;
-    char *include_file = NULL;
+    g_autofree char *profile = NULL;
+    g_autofree char *include_file = NULL;
     off_t size;
     bool purged = 0;
 
@@ -1511,7 +1481,7 @@ main(int argc, char **argv)
         if (ctl->cmd == 'D')
             unlink(include_file);
     } else if (ctl->cmd == 'c' || ctl->cmd == 'r') {
-        char *included_files = NULL;
+        g_autofree char *included_files = NULL;
         g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 
         if (ctl->cmd == 'c' && virFileExists(profile))
@@ -1573,7 +1543,7 @@ main(int argc, char **argv)
 
         /* create the profile from TEMPLATE */
         if (ctl->cmd == 'c' || purged) {
-            char *tmp = NULL;
+            g_autofree char *tmp = NULL;
 #if defined(WITH_APPARMOR_3)
             const char *ifexists = "if exists ";
 #else
@@ -1591,7 +1561,6 @@ main(int argc, char **argv)
                 vah_error(ctl, 0, _("could not create profile"));
                 unlink(include_file);
             }
-            VIR_FREE(tmp);
         }
 
         if (rc == 0 && !ctl->dryrun) {
@@ -1607,14 +1576,9 @@ main(int argc, char **argv)
                     unlink(profile);
             }
         }
-      cleanup:
-        VIR_FREE(included_files);
     }
-
+ cleanup:
     vahDeinit(ctl);
 
-    VIR_FREE(profile);
-    VIR_FREE(include_file);
-
     exit(rc == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }
-- 
2.34.1



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux