At a number of places I noticed a following combination of functions
build_path;
strcat;
Realizing that the way build_path is designed is not very conducive
to such a scenario, and also requires memory to have been pre-allocated
which caused a whole lot of other issues, such as using statically sized
arrays which could overflow, not giving us the opportunity to reallocate,
it looks like to have this sort of a function is a better option. We can
now have dynamically allocated strings which will help us move from static
arrays to dynamic arrays.
As of now, only build tested
Changes from v1
1. Used vasprintf as pointed out by Dan Smith. Minimal testing shows the
usage is correct, but would require closer review
2. Corrected some other bugs which were pointed out in testing
3. Minimally tested using the test cases Sudhir has submitted.
Signed-off-by: Dhaval Giani <[EMAIL PROTECTED]>
---
api.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 125 insertions(+), 29 deletions(-)
Index: trunk/api.c
===================================================================
--- trunk.orig/api.c 2008-11-29 11:24:38.000000000 +0530
+++ trunk/api.c 2008-11-29 11:24:42.000000000 +0530
@@ -40,6 +40,7 @@
#include <ctype.h>
#include <pwd.h>
#include <libgen.h>
+#include <stdarg.h>
#ifndef PACKAGE_VERSION
#define PACKAGE_VERSION 0.01
@@ -650,7 +651,68 @@ static inline pid_t cg_gettid()
return syscall(__NR_gettid);
}
+/*
+ * Always take the mount table lock before calling in here.
+ */
+static char *cg_build_path_var(char *name, char *type, const char *fmt, ...)
+{
+ char *path = NULL, *tmp = NULL, *tmp2 = NULL;
+ int i, size, ret;
+ bool found = false;
+ va_list args;
+
+ for (i = 0; cg_mount_table[i].name[0] != '\0'; i++) {
+ if (strcmp(cg_mount_table[i].name, type) == 0) {
+ found = true;
+ if (name)
+ asprintf(&tmp, "%s/%s", cg_mount_table[i].path,
+ name);
+ else
+ asprintf(&tmp, "%s", cg_mount_table[i].path);
+ if (!name)
+ return NULL;
+ break;
+ }
+ }
+ /*
+ * If !found, then tmp never got allocated and so we don't need
+ * to free it
+ */
+ if (!found)
+ goto err;
+
+ if (!fmt)
+ goto err;
+
+ size = 1024;
+ tmp2 = calloc(1, 1024);
+
+ if (!tmp2)
+ goto err;
+
+ va_start(args, fmt);
+ ret = vasprintf(&tmp2, fmt, args);
+ va_end(args);
+
+ if (ret < 0)
+ goto err;
+
+ ret = asprintf(&path, "%s/%s", tmp, tmp2);
+
+ if (ret < 0)
+ path = NULL;
+err:
+ if (tmp)
+ free(tmp);
+ if (tmp2)
+ free(tmp2);
+
+ /*
+ * Returning path is fine, if asprintf failed, it will return NULL
+ */
+ return path;
+}
/* Call with cg_mount_table_lock taken */
static char *cg_build_path_locked(char *name, char *path, char *type)
{
@@ -1288,7 +1350,7 @@ int cgroup_delete_cgroup(struct cgroup *
{
FILE *delete_tasks, *base_tasks = NULL;
int tids;
- char path[FILENAME_MAX];
+ char *path = NULL;
int error = ECGROUPNOTALLOWED;
int i, ret;
@@ -1304,24 +1366,31 @@ int cgroup_delete_cgroup(struct cgroup *
}
for (i = 0; i < cgroup->index; i++) {
- if (!cg_build_path(cgroup->name, path,
- cgroup->controller[i]->name))
+ pthread_rwlock_rdlock(&cg_mount_table_lock);
+ path = cg_build_path_var(cgroup->name,
+ cgroup->controller[i]->name, "../tasks");
+ pthread_rwlock_unlock(&cg_mount_table_lock);
+
+ if (!path)
continue;
- strncat(path, "../tasks", sizeof(path) - strlen(path));
base_tasks = fopen(path, "w");
+ free(path);
+ path = NULL;
if (!base_tasks)
goto open_err;
- if (!cg_build_path(cgroup->name, path,
- cgroup->controller[i]->name)) {
- fclose(base_tasks);
- continue;
- }
+ pthread_rwlock_rdlock(&cg_mount_table_lock);
+ path = cg_build_path_var(cgroup->name,
+ cgroup->controller[i]->name, "tasks");
+ pthread_rwlock_unlock(&cg_mount_table_lock);
- strncat(path, "tasks", sizeof(path) - strlen(path));
+ if (!path)
+ continue;
delete_tasks = fopen(path, "r");
+ free(path);
+ path = NULL;
if (!delete_tasks) {
fclose(base_tasks);
goto open_err;
@@ -1337,6 +1406,16 @@ int cgroup_delete_cgroup(struct cgroup *
fclose(delete_tasks);
fclose(base_tasks);
+ if (path)
+ free(path);
+
+ path = calloc(FILENAME_MAX, 1);
+
+ if (path) {
+ error = ECGOTHER;
+ goto open_err;
+ }
+
if (!cg_build_path(cgroup->name, path,
cgroup->controller[i]->name))
continue;
@@ -1367,15 +1446,18 @@ open_err:
static char *cg_rd_ctrl_file(char *subsys, char *cgroup, char *file)
{
char *value;
- char path[FILENAME_MAX];
+ char *path;
FILE *ctrl_file;
int ret;
- if (!cg_build_path_locked(cgroup, path, subsys))
+ path = cg_build_path_var(cgroup, subsys, "%s", file);
+
+ if (!path)
return NULL;
- strncat(path, file, sizeof(path) - strlen(path));
ctrl_file = fopen(path, "r");
+
+ free(path);
if (!ctrl_file)
return NULL;
@@ -1408,7 +1490,7 @@ static int cgroup_fill_cgc(struct dirent
char *ctrl_file;
char *ctrl_value = NULL;
char *d_name;
- char path[FILENAME_MAX+1];
+ char *path = NULL;
char *buffer;
int error = 0;
struct stat stat_buffer;
@@ -1426,8 +1508,14 @@ static int cgroup_fill_cgc(struct dirent
* some sort of a flag, but this is fine for now.
*/
- cg_build_path_locked(cgroup->name, path, cg_mount_table[index].name);
- strncat(path, d_name, sizeof(path) - strlen(path));
+ pthread_rwlock_rdlock(&cg_mount_table_lock);
+ path = cg_build_path_var(cgroup->name, cg_mount_table[index].name,
+ "%s", d_name);
+ pthread_rwlock_unlock(&cg_mount_table_lock);
+ if (!path) {
+ error = ECGOTHER;
+ goto fill_error;
+ }
error = stat(path, &stat_buffer);
@@ -1468,6 +1556,8 @@ static int cgroup_fill_cgc(struct dirent
}
}
fill_error:
+ if (path)
+ free(path);
if (ctrl_value)
free(ctrl_value);
free(d_name);
@@ -1483,7 +1573,7 @@ fill_error:
int cgroup_get_cgroup(struct cgroup *cgroup)
{
int i;
- char path[FILENAME_MAX];
+ char *path = NULL;
DIR *dir;
struct dirent *ctrl_dir;
char *control_path;
@@ -1511,24 +1601,27 @@ int cgroup_get_cgroup(struct cgroup *cgr
struct stat stat_buffer;
int path_len;
- if (!cg_build_path_locked(NULL, path,
- cg_mount_table[i].name))
- continue;
+ pthread_rwlock_rdlock(&cg_mount_table_lock);
+ path = cg_build_path_var(NULL, cg_mount_table[i].name, "%s",
+ cgroup->name);
+ pthread_rwlock_unlock(&cg_mount_table_lock);
- path_len = strlen(path);
- strncat(path, cgroup->name, FILENAME_MAX - path_len - 1);
+ if (!path) {
+ error = ECGOTHER;
+ goto unlock_error;
+ }
if (access(path, F_OK))
continue;
- if (!cg_build_path_locked(cgroup->name, path,
- cg_mount_table[i].name)) {
- /*
- * This fails when the cgroup does not exist
- * for that controller.
- */
+ free(path);
+
+ pthread_rwlock_rdlock(&cg_mount_table_lock);
+ path = cg_build_path_var(cgroup->name, cg_mount_table[i].name,
+ NULL);
+ pthread_rwlock_unlock(&cg_mount_table_lock);
+ if (!path)
continue;
- }
/*
* Get the uid and gid information
@@ -1560,6 +1653,7 @@ int cgroup_get_cgroup(struct cgroup *cgr
}
dir = opendir(path);
+ free(path);
if (!dir) {
error = ECGOTHER;
goto unlock_error;
@@ -1592,6 +1686,8 @@ int cgroup_get_cgroup(struct cgroup *cgr
unlock_error:
pthread_rwlock_unlock(&cg_mount_table_lock);
+ if (path)
+ free(path);
/*
* XX: Need to figure out how to cleanup? Cleanup just the stuff
* we added, or the whole structure.
--
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
