Hi Dhaval, I have attached a new patch.
On 12/10/2010 03:45 PM, Dhaval Giani wrote: > On Fri, Dec 10, 2010 at 10:47 AM, Ciju Rajan K<[email protected]> > wrote: >> Hi All, >> >> When we invoke cgroup_get_cgroup() to get the cgroup meta data, the admin_id >> and admin_gid are not displayed correctly. This is because cgroup_fill_cgc() >> does not differentiate between the cgroup control files and tasks file. So >> cgroup->control_uid and cgroup->control_gid fields are getting populated >> with the uid and gid of tasks file. >> >> This patch fixes this problem by adding a check in the cgroup_fill_cgc() >> function to see if the file is a 'tasks' file or not. >> > > As we discussed on IRC, I am not sure this is the right approach. Two > requests, > Changed the approach. This should address your concerns > 1. Extensively test with groups which have tasks as a substring in the > their name. Done! It is working fine with "*tasks*" in directory names. > 2. Comment why strstr is safe. If you cannot prove it, I would rather > use a strncmp for the last 6 chars with /tasks. Done! > > Also, could you please inline the patch? Makes it a bit easier to > comment on it ;-) Done :-) -Ciju --- libcgroup-0.37.rc1/src/api.c.orig 2010-12-10 15:01:36.000000000 +0530 +++ libcgroup-0.37.rc1/src/api.c 2010-12-10 19:34:29.000000000 +0530 @@ -1914,6 +1914,8 @@ static int cgroup_fill_cgc(struct dirent char *ctrl_value = NULL; char *d_name = NULL; char path[FILENAME_MAX+1]; + char *tmp_path = NULL; + int tmp_len = 0; char *buffer = NULL; int error = 0; struct stat stat_buffer; @@ -1941,8 +1943,33 @@ static int cgroup_fill_cgc(struct dirent goto fill_error; } - cgroup->control_uid = stat_buffer.st_uid; - cgroup->control_gid = stat_buffer.st_gid; + /* + * We have already stored the tasks_uid & tasks_gid. + * This check is to avoid the overwriting of the values + * stored in control_uid & cotrol_gid. tasks file will + * have the uid and gid of the user who is capable of + * putting a task to this cgroup. control_uid and control_gid + * is meant for the users who are capable of managing the + * cgroup shares. + * + * The strstr() function will return the pointer to the + * beginning of the sub string "/tasks". + */ + tmp_len = strlen(path) - strlen("/tasks"); + + /* + * tmp_path would be pointing to the last six characters + */ + tmp_path = (char *)path + tmp_len; + + /* + * Checking to see, if this is actually a 'tasks' file + * We need to compare the last 6 bytes + */ + if (strcmp(tmp_path, "/tasks")){ + cgroup->control_uid = stat_buffer.st_uid; + cgroup->control_gid = stat_buffer.st_gid; + } ctrl_name = strtok_r(d_name, ".", &buffer); ------------------------------------------------------------------------------ _______________________________________________ Libcg-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/libcg-devel
