api.c: refactor cgroup_init()

Refactor cgroup_init() by abstracting freeing of previous cg_mount_table,
populating controller by reading /proc/cgroups file and populating
mount points into its own function.

Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
TJH: Fix minor typo in commit message

diff --git a/src/api.c b/src/api.c
index b445dd77323ae60c698fac3383dc76e98c56beb1..d6867ec768f84f0e06cc4118c5757a1649419d9b 100644 (file)
--- a/src/api.c
+++ b/src/api.c
@@ -1292,56 +1292,48 @@ out:
        return ret;
 }
 
-/**
- * cgroup_init(), initializes the MOUNT_POINT.
- *
- * This code is theoretically thread safe now. Its not really tested
- * so it can blow up. If does for you, please let us know with your
- * test case and we can really make it thread safe.
- *
+/*
+ * Free global variables filled by previous cgroup_init(). This function
+ * should be called with cg_mount_table_lock taken.
  */
-int cgroup_init(void)
+static void cgroup_free_cg_mount_table(void)
 {
-       static char *controllers[CG_CONTROLLER_MAX];
-       int hierarchy, num_cgroups, enabled;
-       char mntent_buffer[4 * FILENAME_MAX];
-       char subsys_name[FILENAME_MAX];
-       struct mntent *temp_ent = NULL;
-       struct mntent *ent = NULL;
-       FILE *proc_cgroup = NULL;
-       FILE *proc_mount = NULL;
-       int found_mnt = 0;
-       char *buf = NULL;
-       int ret = 0;
-       int i = 0;
-       int err;
-
-       cgroup_set_default_logger(-1);
-
-       pthread_rwlock_wrlock(&cg_mount_table_lock);
+       struct cg_mount_point *mount, *tmp;
+       int i;
 
-       /* Free global variables filled by previous cgroup_init() */
        for (i = 0; cg_mount_table[i].name[0] != '\0'; i++) {
-               struct cg_mount_point *mount = cg_mount_table[i].mount.next;
+               mount = cg_mount_table[i].mount.next;
 
                while (mount) {
-                       struct cg_mount_point *tmp = mount;
-
+                       tmp = mount;
                        mount = mount->next;
                        free(tmp);
                }
        }
-       memset(&cg_mount_table, 0, sizeof(cg_mount_table));
 
+       memset(&cg_mount_table, 0, sizeof(cg_mount_table));
        memset(&cg_cgroup_v2_mount_path, 0, sizeof(cg_cgroup_v2_mount_path));
+}
 
-       proc_cgroup = fopen("/proc/cgroups", "re");
+/*
+ * Reads /proc/cgroups and populates the controllers/subsys_name. This function
+ * should be called with cg_mount_table_lock taken.
+ */
+static int cgroup_populate_controllers(char *controllers[CG_CONTROLLER_MAX])
+{
+       int hierarchy, num_cgroups, enabled;
+       char subsys_name[FILENAME_MAX];
+       FILE *proc_cgroup;
+       int ret = 0;
+       int i, err;
+       char *buf;
 
+       proc_cgroup = fopen("/proc/cgroups", "re");
        if (!proc_cgroup) {
                cgroup_err("cannot open /proc/cgroups: %s\n", strerror(errno));
                last_errno = errno;
                ret = ECGOTHER;
-               goto unlock_exit;
+               goto err;
        }
 
        /*
@@ -1352,16 +1344,15 @@ int cgroup_init(void)
        if (!buf) {
                last_errno = errno;
                ret = ECGOTHER;
-               goto unlock_exit;
+               goto err;
        }
+
        if (!fgets(buf, LL_MAX, proc_cgroup)) {
-               free(buf);
                cgroup_err("cannot read /proc/cgroups: %s\n", strerror(errno));
                last_errno = errno;
                ret = ECGOTHER;
-               goto unlock_exit;
+               goto err;
        }
-       free(buf);
 
        i = 0;
        while (!feof(proc_cgroup)) {
@@ -1369,24 +1360,59 @@ int cgroup_init(void)
                             &hierarchy, &num_cgroups, &enabled);
                if (err < 0)
                        break;
+
                controllers[i] = strdup(subsys_name);
+               if (controllers[i] == NULL) {
+                       last_errno = errno;
+                       ret = ECGOTHER;
+                       break;
+               }
                i++;
        }
-       controllers[i] = NULL;
+
+err:
+       if (proc_cgroup)
+               fclose(proc_cgroup);
+
+       if (buf)
+               free(buf);
+
+       if (ret != 0) {
+               for (i = 0; controllers[i]; i++) {
+                       free(controllers[i]);
+                       controllers[i] = NULL;
+               }
+       }
+
+       return ret;
+}
+
+/*
+ * Reads /proc/mounts and populates the cgroup v1/v2 mount points into the
+ * global cg_mount_table. This function should be called with
+ * cg_mount_table_lock taken.
+ */
+static int cgroup_populate_mount_points(char *controllers[CG_CONTROLLER_MAX])
+{
+       char mntent_buffer[4 * FILENAME_MAX];
+       struct mntent *temp_ent, *ent;
+       int found_mnt = 0;
+       FILE *proc_mount;
+       int ret = 0;
 
        proc_mount = fopen("/proc/mounts", "re");
        if (proc_mount == NULL) {
                cgroup_err("cannot open /proc/mounts: %s\n", strerror(errno));
                last_errno = errno;
                ret = ECGOTHER;
-               goto unlock_exit;
+               goto err;
        }
 
        temp_ent = (struct mntent *) malloc(sizeof(struct mntent));
        if (!temp_ent) {
                last_errno = errno;
                ret = ECGOTHER;
-               goto unlock_exit;
+               goto err;
        }
 
        while ((ent = getmntent_r(proc_mount, temp_ent, mntent_buffer,
@@ -1396,42 +1422,71 @@ int cgroup_init(void)
                        ret = cgroup_process_v1_mnt(controllers, ent,
                                                    &found_mnt);
                        if (ret)
-                               goto unlock_exit;
-               } else if (strcmp(ent->mnt_type, "cgroup2") == 0) {
+                               goto err;
+
+                       continue;
+               }
+
+               if (strcmp(ent->mnt_type, "cgroup2") == 0) {
                        ret = cgroup_process_v2_mnt(ent, &found_mnt);
                        if (ret == ECGEOF) {
-                               /* The controllers file was empty.  Ignore and
-                                * move on.
-                                */
+                       /*
+                        * The controllers file was empty.  Ignore and move on.
+                        */
                                ret = 0;
                                continue;
-                       } else if (ret) {
-                               goto unlock_exit;
                        }
+
+                       if (ret)
+                               goto err;
                }
        }
 
-       if (!found_mnt) {
-               cg_mount_table[0].name[0] = '\0';
+       if (!found_mnt)
                ret = ECGROUPNOTMOUNTED;
-               goto unlock_exit;
-       }
-
-       found_mnt++;
-       cg_mount_table[found_mnt].name[0] = '\0';
-
-       cgroup_initialized = 1;
-
-unlock_exit:
-       if (proc_cgroup)
-               fclose(proc_cgroup);
 
+err:
        if (proc_mount)
                fclose(proc_mount);
 
        if (temp_ent)
                free(temp_ent);
 
+       return ret;
+}
+
+/**
+ * cgroup_init(), initializes the MOUNT_POINT.
+ *
+ * This code is theoretically thread safe now. Its not really tested
+ * so it can blow up. If does for you, please let us know with your
+ * test case and we can really make it thread safe.
+ *
+ */
+int cgroup_init(void)
+{
+       static char *controllers[CG_CONTROLLER_MAX];
+       int ret = 0;
+       int i;
+
+       cgroup_set_default_logger(-1);
+
+       pthread_rwlock_wrlock(&cg_mount_table_lock);
+
+       /* Free global variables filled by previous cgroup_init() */
+       cgroup_free_cg_mount_table();
+
+       ret = cgroup_populate_controllers(controllers);
+       if (ret)
+               goto unlock_exit;
+
+       ret = cgroup_populate_mount_points(controllers);
+       if (ret)
+               goto unlock_exit;
+
+       cgroup_initialized = 1;
+
+unlock_exit:
        for (i = 0; controllers[i]; i++) {
                free(controllers[i]);
                controllers[i] = NULL;