On 06/16/2011 01:53 PM, Michal Hocko wrote: > On Thu 16-06-11 12:51:39, Jan Safranek wrote: >> On 06/16/2011 09:35 AM, Michal Hocko wrote: >>> Wouldn't something like function pointer parameter ((*exclude_filer)(const >>> char *)) be better? >> >> Oops, my patch is wrong, I forgot cg_chmod_recursive which should not be >> affected by my change. Instead of callback I would propose simple list of >> files to ignore, see updated patch below. > > Why do you think that the list is any better? > To me it looks that the array approach bloats the function call somehow > more. Function approach is also more flexible (e.g. consider that there > will be a need to configure which files permissions should be changed by > regexp.).
We have two callers of the function now and I don't there will be more of them. And the function call would be either with char ** with list of file names or int (*filter)(char*) as callback. I don't see much difference in bloating function prototype. Actually, the second looks worse to me. Anyway, this is internal function and can be changed anytime, when we consider we need regular expression or something more advanced than list of file names. > >> >> 8<--- >> >> When cgroup_create_cgroup() is called with different control_fperm and >> task_fperm, libcgroup first changes permissions of *all* files to match >> the control_fperm, which includes the also the tasks file and it might >> loose some permission bits. The tasks file is then modulated by >> control_fperm, but because umask-style of control_fperm, it might get >> lower permissions that users would expect. >> >> Therefore 'tasks' file must be skipped when using control_fperm. >> >> Changelog since v1: >> - use list of ignored files, cg_chmod_recursive_controller is called from >> cg_chmod_recursive, which should chmod all files, incl. 'tasks'. >> >> Signed-off-by: Jan Safranek <jsafr...@redhat.com> >> --- >> >> src/api.c | 28 ++++++++++++++++++++++------ >> 1 files changed, 22 insertions(+), 6 deletions(-) >> >> diff --git a/src/api.c b/src/api.c >> index fd26c6d..a02595a 100644 >> --- a/src/api.c >> +++ b/src/api.c >> @@ -120,6 +120,8 @@ const char const *cgroup_strerror_codes[] = { >> "Value setting does not succeed", >> }; >> >> +static const char const *cgroup_control_files[] = { "tasks", NULL }; > > The name is misleading. This is not a control file. Ok, I'll rename it. >> + >> static int cg_chown_file(FTS *fts, FTSENT *ent, uid_t owner, gid_t group) >> { >> int ret = 0; >> @@ -240,17 +242,20 @@ int cg_chmod_file(FTS *fts, FTSENT *ent, mode_t >> dir_mode, >> } >> >> >> -/* >> - * TODO: Need to decide a better place to put this function. >> +/** >> + * Changes permissions of all directories and control files (i.e. all >> + * files except files named in ignore_list. The list must be terminated with >> + * NULL. >> */ >> static int cg_chmod_recursive_controller(char *path, mode_t dir_mode, >> int dirm_change, mode_t file_mode, int filem_change, >> - int owner_is_umask) >> + int owner_is_umask, const char const **ignore_list) >> { >> int ret = 0; >> int final_ret =0; >> FTS *fts; >> char *fts_path[2]; >> + int i, ignored; >> >> fts_path[0] = path; >> fts_path[1] = NULL; >> @@ -273,8 +278,19 @@ static int cg_chmod_recursive_controller(char *path, >> mode_t dir_mode, >> } >> break; >> } >> + ignored = 0; >> + if (ignore_list != NULL) >> + for (i = 0; ignore_list[i] != NULL; i++) >> + if (!strcmp(ignore_list[i], ent->fts_name)) { >> + ignored = 1; >> + break; >> + } >> + if (ignored) >> + continue; >> + >> ret = cg_chmod_file(fts, ent, dir_mode, dirm_change, >> - file_mode, filem_change, owner_is_umask); >> + file_mode, filem_change, >> + owner_is_umask); >> if (ret) { >> last_errno = errno; >> final_ret = ECGOTHER; > ------------------------------------------------------------------------------ EditLive Enterprise is the world's most technically advanced content authoring tool. Experience the power of Track Changes, Inline Image Editing and ensure content is compliant with Accessibility Checking. http://p.sf.net/sfu/ephox-dev2dev _______________________________________________ Libcg-devel mailing list Libcg-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libcg-devel