From: Kamalesh Babulal Date: Sat, 26 Mar 2022 13:40:43 +0000 (+0530) Subject: api.c: refactor cgroup_init() X-Git-Tag: v3.0~97 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=339d6274bc3526f9f5af0365e80c8f2bb6976243;p=thirdparty%2Flibcgroup.git 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 Signed-off-by: Tom Hromatka TJH: Fix minor typo in commit message --- diff --git a/src/api.c b/src/api.c index b445dd77..d6867ec7 100644 --- 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;