Some of the cleanups possible are obvious.
1. Change usage of strcat to strncat
2. Change usage of tge following type
char *s = malloc();
strcpy(s, "somestring");
strcat(s, "someotherstring");
to something more easily understandble such as
asprintf(&s, "%s%s", somestring, someotherstring);
Changes from v1:
1. Correct the error handling of asprintf as pointed out by Dan Smith
TODO:
1. Figure out what is the correct value of n for cg_build_path_locked
Signed-off-by: Dhaval Giani <[EMAIL PROTECTED]>
---
api.c | 101 +++++++++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 64 insertions(+), 37 deletions(-)
Index: trunk/api.c
===================================================================
--- trunk.orig/api.c 2008-11-10 19:20:24.000000000 +0530
+++ trunk/api.c 2008-11-29 11:24:38.000000000 +0530
@@ -536,8 +536,7 @@ int cgroup_init()
&hierarchy, &num_cgroups, &enabled);
if (err < 0)
break;
- controllers[i] = (char *)malloc(strlen(subsys_name) + 1);
- strcpy(controllers[i], subsys_name);
+ controllers[i] = strdup(subsys_name);
i++;
}
controllers[i] = NULL;
@@ -600,6 +599,11 @@ int cgroup_init()
unlock_exit:
pthread_rwlock_unlock(&cg_mount_table_lock);
+
+ for (i = 0; controllers[i] && i < CG_CONTROLLER_MAX; i++) {
+ free(controllers[i]);
+ controllers[i] = NULL;
+ }
return ret;
}
@@ -652,12 +656,13 @@ static char *cg_build_path_locked(char *
{
int i;
for (i = 0; cg_mount_table[i].name[0] != '\0'; i++) {
+ /*
+ * XX: Change to snprintf once you figure what n should be
+ */
if (strcmp(cg_mount_table[i].name, type) == 0) {
- strcpy(path, cg_mount_table[i].path);
- strcat(path, "/");
+ sprintf(path, "%s/", cg_mount_table[i].path);
if (name) {
- strcat(path, name);
- strcat(path, "/");
+ sprintf(path, "%s%s/", path, name);
}
return path;
}
@@ -700,7 +705,7 @@ int cgroup_attach_task_pid(struct cgroup
if (!cg_build_path_locked(NULL, path,
cg_mount_table[i].name))
continue;
- strcat(path, "/tasks");
+ strncat(path, "/tasks", sizeof(path) - strlen(path));
tasks = fopen(path, "w");
if (!tasks) {
@@ -746,7 +751,7 @@ int cgroup_attach_task_pid(struct cgroup
cgroup->controller[i]->name))
continue;
- strcat(path, "/tasks");
+ strncat(path, "/tasks", sizeof(path) - strlen(path));
tasks = fopen(path, "w");
if (!tasks) {
@@ -910,7 +915,7 @@ static int cg_set_control_value(char *pa
while (*(path+len) != '/')
len--;
*(path+len+1) = '\0';
- strcat(path, "tasks");
+ strncat(path, "tasks", sizeof(path) - strlen(path));
control_file = fopen(path, "r");
if (!control_file) {
if (errno == ENOENT)
@@ -940,9 +945,10 @@ static int cg_set_control_value(char *pa
int cgroup_modify_cgroup(struct cgroup *cgroup)
{
- char path[FILENAME_MAX], base[FILENAME_MAX];
+ char *path, base[FILENAME_MAX];
int i;
int error;
+ int ret;
if (!cgroup_initialized)
return ECGROUPNOTINITIALIZED;
@@ -958,24 +964,32 @@ int cgroup_modify_cgroup(struct cgroup *
}
}
- strcpy(path, base);
- for (i = 0; i < cgroup->index; i++, strcpy(path, base)) {
+ for (i = 0; i < cgroup->index; i++) {
int j;
if (!cg_build_path(cgroup->name, base,
cgroup->controller[i]->name))
continue;
- strcpy(path, base);
- for (j = 0; j < cgroup->controller[i]->index;
- j++, strcpy(path, base)) {
- strcat(path, cgroup->controller[i]->values[j]->name);
+ for (j = 0; j < cgroup->controller[i]->index; j++) {
+ ret = asprintf(&path, "%s%s", base,
+ cgroup->controller[i]->values[j]->name);
+ if (ret < 0) {
+ error = ECGOTHER;
+ goto err;
+ }
error = cg_set_control_value(path,
cgroup->controller[i]->values[j]->value);
+ free(path);
+ path = NULL;
if (error)
goto err;
}
}
+ if (path)
+ free(path);
return 0;
err:
+ if (path)
+ free(path);
return error;
}
@@ -1062,10 +1076,11 @@ err:
*/
int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership)
{
- char *fts_path[2], base[FILENAME_MAX], *path;
+ char *fts_path[2], *base = NULL, *path;
int i, j, k;
int error = 0;
int retval = 0;
+ int ret;
if (!cgroup_initialized)
return ECGROUPNOTINITIALIZED;
@@ -1090,8 +1105,6 @@ int cgroup_create_cgroup(struct cgroup *
* data structure. If not, we fail.
*/
for (k = 0; k < cgroup->index; k++) {
- path[0] = '\0';
-
if (!cg_build_path(cgroup->name, path,
cgroup->controller[k]->name))
continue;
@@ -1100,7 +1113,12 @@ int cgroup_create_cgroup(struct cgroup *
if (error)
goto err;
- strcpy(base, path);
+ base = strdup(path);
+
+ if (!base) {
+ error = ECGOTHER;
+ goto err;
+ }
if (!ignore_ownership)
error = cg_chown_recursive(fts_path,
@@ -1109,9 +1127,14 @@ int cgroup_create_cgroup(struct cgroup *
if (error)
goto err;
- for (j = 0; j < cgroup->controller[k]->index; j++,
- strcpy(path, base)) {
- strcat(path, cgroup->controller[k]->values[j]->name);
+ for (j = 0; j < cgroup->controller[k]->index; j++) {
+ free(path);
+ ret = asprintf(&path, "%s%s", base,
+ cgroup->controller[k]->values[j]->name);
+ if (ret < 0) {
+ error = ECGOTHER;
+ goto err;
+ }
error = cg_set_control_value(path,
cgroup->controller[k]->values[j]->value);
/*
@@ -1130,19 +1153,26 @@ int cgroup_create_cgroup(struct cgroup *
}
if (!ignore_ownership) {
- strcpy(path, base);
- strcat(path, "/tasks");
+ free(path);
+ ret = asprintf(&path, "%s/tasks", base);
+ if (ret < 0) {
+ error = ECGOTHER;
+ goto err;
+ }
error = chown(path, cgroup->tasks_uid,
cgroup->tasks_gid);
if (error) {
- error = ECGFAIL;
+ error = ECGOTHER;
goto err;
}
}
}
err:
- free(path);
+ if (path)
+ free(path);
+ if (base)
+ free(base);
if (retval && !error)
error = retval;
return error;
@@ -1277,7 +1307,7 @@ int cgroup_delete_cgroup(struct cgroup *
if (!cg_build_path(cgroup->name, path,
cgroup->controller[i]->name))
continue;
- strcat(path, "../tasks");
+ strncat(path, "../tasks", sizeof(path) - strlen(path));
base_tasks = fopen(path, "w");
if (!base_tasks)
@@ -1289,7 +1319,7 @@ int cgroup_delete_cgroup(struct cgroup *
continue;
}
- strcat(path, "tasks");
+ strncat(path, "tasks", sizeof(path) - strlen(path));
delete_tasks = fopen(path, "r");
if (!delete_tasks) {
@@ -1344,7 +1374,7 @@ static char *cg_rd_ctrl_file(char *subsy
if (!cg_build_path_locked(cgroup, path, subsys))
return NULL;
- strcat(path, file);
+ strncat(path, file, sizeof(path) - strlen(path));
ctrl_file = fopen(path, "r");
if (!ctrl_file)
return NULL;
@@ -1397,7 +1427,7 @@ static int cgroup_fill_cgc(struct dirent
*/
cg_build_path_locked(cgroup->name, path, cg_mount_table[index].name);
- strcat(path, d_name);
+ strncat(path, d_name, sizeof(path) - strlen(path));
error = stat(path, &stat_buffer);
@@ -1458,6 +1488,7 @@ int cgroup_get_cgroup(struct cgroup *cgr
struct dirent *ctrl_dir;
char *control_path;
int error;
+ int ret;
if (!cgroup_initialized) {
/* ECGROUPNOTINITIALIZED */
@@ -1503,17 +1534,13 @@ int cgroup_get_cgroup(struct cgroup *cgr
* Get the uid and gid information
*/
- control_path = malloc(strlen(path) + strlen("tasks") + 1);
+ ret = asprintf(&control_path, "%s/tasks", path);
- if (!control_path) {
+ if (ret < 0) {
error = ECGOTHER;
goto unlock_error;
}
- strcpy(control_path, path);
-
- strcat(control_path, "tasks");
-
if (stat(control_path, &stat_buffer)) {
free(control_path);
error = ECGOTHER;
--
regards,
Dhaval
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
