[lxc-devel] [PATCH] cleanup fds, memory in lxc_cgroup_load_meta2()
Dwight Engen
dwight.engen at oracle.com
Mon Sep 16 23:55:06 UTC 2013
There are fd leaks in lxc_cgroup_load_meta2() in particular in the success
case. This change attempts to ensure resources are free'd/close'd, but it
is possible there are still some error cases where leaks occur.
Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
---
Hi Christian,
These changes fix problems I was seeing with valgrind, but they may not be
how you would do it, so feel free to rework them if I've messed up your
code too bad :) The command I was using:
valgrind --leak-check=full --track-fds=yes ./lxc-test-concurrent -i 2 -j 1
src/lxc/cgroup.c | 83 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 48 insertions(+), 35 deletions(-)
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 9417e77..101998b 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -124,7 +124,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
size_t token_capacity = 0;
char *line = NULL;
size_t sz = 0;
- int r, saved_errno = 0;
+ int r, ret = ENOMEM;
/* if the subsystem whitelist is not specified, include all
* hierarchies that contain kernel subsystems by default but
@@ -149,8 +149,10 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
/* Step 1: determine all kernel subsystems */
proc_cgroups = fopen_cloexec("/proc/cgroups", "r");
- if (!proc_cgroups)
- goto out_error;
+ if (!proc_cgroups) {
+ ret = errno;
+ goto out1;
+ }
while (getline(&line, &sz, proc_cgroups) != -1) {
char *tab1;
@@ -179,10 +181,10 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
r = lxc_grow_array((void ***)&kernel_subsystems, &kernel_subsystems_capacity, kernel_subsystems_count + 1, 12);
if (r < 0)
- goto out_error;
+ goto out2;
kernel_subsystems[kernel_subsystems_count] = strdup(line);
if (!kernel_subsystems[kernel_subsystems_count])
- goto out_error;
+ goto out2;
kernel_subsystems_count++;
}
@@ -198,8 +200,10 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
* /proc/self is not valid, we try /proc/1/cgroup... */
if (!proc_self_cgroup)
proc_self_cgroup = fopen_cloexec("/proc/1/cgroup", "r");
- if (!proc_self_cgroup)
- goto out_error;
+ if (!proc_self_cgroup) {
+ ret = errno;
+ goto out2;
+ }
while (getline(&line, &sz, proc_self_cgroup) != -1) {
/* file format: hierarchy:subsystems:group,
@@ -234,25 +238,25 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
*/
r = lxc_grow_array((void ***)&meta_data->hierarchies, &hierarchy_capacity, hierarchy_number + 1, 12);
if (r < 0)
- goto out_error;
+ goto out3;
meta_data->maximum_hierarchy = hierarchy_number;
}
/* this shouldn't happen, we had this already */
if (meta_data->hierarchies[hierarchy_number])
- goto out_error;
+ goto out3;
h = calloc(1, sizeof(struct cgroup_hierarchy));
if (!h)
- goto out_error;
+ goto out3;
meta_data->hierarchies[hierarchy_number] = h;
h->index = hierarchy_number;
h->subsystems = lxc_string_split_and_trim(colon1, ',');
if (!h->subsystems)
- goto out_error;
+ goto out3;
/* see if this hierarchy should be considered */
if (!all_kernel_subsystems || !all_named_subsystems) {
for (p = h->subsystems; *p; p++) {
@@ -283,8 +287,10 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
* /proc/self is not valid, we try /proc/1/cgroup... */
if (!proc_self_mountinfo)
proc_self_mountinfo = fopen_cloexec("/proc/1/mountinfo", "r");
- if (!proc_self_mountinfo)
- goto out_error;
+ if (!proc_self_mountinfo) {
+ ret = errno;
+ goto out3;
+ }
while (getline(&line, &sz, proc_self_mountinfo) != -1) {
char *token, *saveptr = NULL;
@@ -299,7 +305,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
for (i = 0; (token = strtok_r(line, " ", &saveptr)); line = NULL) {
r = lxc_grow_array((void ***)&tokens, &token_capacity, i + 1, 64);
if (r < 0)
- goto out_error;
+ goto out4;
tokens[i++] = token;
}
@@ -335,7 +341,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
subsystems = subsystems_from_mount_options(tokens[j + 3], kernel_subsystems);
if (!subsystems)
- goto out_error;
+ goto out4;
h = NULL;
for (k = 1; k <= meta_data->maximum_hierarchy; k++) {
@@ -352,12 +358,12 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
r = lxc_grow_array((void ***)&meta_data->mount_points, &mount_point_capacity, mount_point_count + 1, 12);
if (r < 0)
- goto out_error;
+ goto out4;
/* create mount point object */
mount_point = calloc(1, sizeof(*mount_point));
if (!mount_point)
- goto out_error;
+ goto out4;
meta_data->mount_points[mount_point_count++] = mount_point;
@@ -365,7 +371,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
mount_point->mount_point = strdup(tokens[4]);
mount_point->mount_prefix = strdup(tokens[3]);
if (!mount_point->mount_point || !mount_point->mount_prefix)
- goto out_error;
+ goto out4;
mount_point->read_only = !lxc_string_in_list("rw", tokens[5], ',');
if (!strcmp(mount_point->mount_prefix, "/")) {
@@ -381,32 +387,39 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char **subsystem_whitelist)
k = lxc_array_len((void **)h->all_mount_points);
r = lxc_grow_array((void ***)&h->all_mount_points, &h->all_mount_point_capacity, k + 1, 4);
if (r < 0)
- goto out_error;
+ goto out4;
h->all_mount_points[k] = mount_point;
}
/* oops, we couldn't find anything */
if (!meta_data->hierarchies || !meta_data->mount_points) {
- errno = EINVAL;
- goto out_error;
+ ret = EINVAL;
+ goto out4;
}
- return meta_data;
-
-out_error:
- saved_errno = errno;
- if (proc_cgroups)
- fclose(proc_cgroups);
- if (proc_self_cgroup)
- fclose(proc_self_cgroup);
+ ret = 0;
+out4:
if (proc_self_mountinfo)
fclose(proc_self_mountinfo);
- free(line);
- free(tokens);
- lxc_free_array((void **)kernel_subsystems, free);
- lxc_cgroup_put_meta(meta_data);
- errno = saved_errno;
- return NULL;
+out3:
+ if (proc_self_cgroup)
+ fclose(proc_self_cgroup);
+out2:
+ if (proc_cgroups)
+ fclose(proc_cgroups);
+ if (line)
+ free(line);
+ if (tokens)
+ free(tokens);
+out1:
+ if (ret != 0) {
+ lxc_free_array((void **)kernel_subsystems, free);
+ lxc_cgroup_put_meta(meta_data);
+ free(meta_data);
+ meta_data = NULL;
+ }
+ errno = ret;
+ return meta_data;
}
struct cgroup_meta_data *lxc_cgroup_get_meta(struct cgroup_meta_data *meta_data)
--
1.8.1.4
More information about the lxc-devel
mailing list