On 05/19/2011 09:39 PM, Michal Hocko wrote:
> Currently we are using flag parameters to determine whether to use
> directory resp. file permissions. This is rather ugly because we
> need to maintain 2 variables for one purpose while we can use -1 for
> permission values that shouldn't be used.
>
> Signed-off-by: Michal Hocko <mho...@suse.cz>
> ---
> include/libcgroup/groups.h | 8 +++-----
> src/api.c | 11 +++++------
> src/tools/cgcreate.c | 14 +++++---------
> 3 files changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/include/libcgroup/groups.h b/include/libcgroup/groups.h
> index 1060641..c46e261 100644
> --- a/include/libcgroup/groups.h
> +++ b/include/libcgroup/groups.h
> @@ -523,13 +523,11 @@ int cgroup_get_procs(char *name, char *controller,
> pid_t **pids, int *size);
> /**
> * Change permission of files and directories of given group
> * @param cgroup The cgroup which permissions should be changed
> - * @param dir_mode The permission mode of group directory
> - * @param dirm_change Denotes whether the directory change should be done
> - * @param file_mode The permission mode of group files
> - * @param filem_change Denotes whether the directory change should be done
> + * @param dir_mode The permission mode of group directory (-1 for no change)
> + * @param file_mode The permission mode of group files (-1 for no change)
> */
> int cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
> - int dirm_change, mode_t file_mode, int filem_change);
> + mode_t file_mode);
NACK, this changes ABI! As I commented in previous patch, please make
new function. *Sigh* :(.
Otherwise, all 4 patches are very well written, keep up the good work!
Jan
> /**
> * @}
> diff --git a/src/api.c b/src/api.c
> index 311db8d..8be7495 100644
> --- a/src/api.c
> +++ b/src/api.c
> @@ -202,7 +202,7 @@ fail:
> }
>
> int cg_chmod_file(FTS *fts, FTSENT *ent, mode_t dir_mode,
> - int dirm_change, mode_t file_mode, int filem_change)
> + mode_t file_mode)
> {
> int ret = 0;
> const char *filename = fts->fts_path;
> @@ -217,14 +217,14 @@ int cg_chmod_file(FTS *fts, FTSENT *ent, mode_t
> dir_mode,
> case FTS_DC:
> case FTS_DNR:
> case FTS_DP:
> - if (dirm_change)
> + if (dir_mode != -1U)
> ret = cg_chmod_path(filename, dir_mode);
> break;
> case FTS_F:
> case FTS_NSOK:
> case FTS_NS:
> case FTS_DEFAULT:
> - if (filem_change)
> + if (file_mode != -1U)
> ret = cg_chmod_path(filename, file_mode);
> break;
> }
> @@ -236,7 +236,7 @@ int cg_chmod_file(FTS *fts, FTSENT *ent, mode_t dir_mode,
> * TODO: Need to decide a better place to put this function.
> */
> int cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
> - int dirm_change, mode_t file_mode, int filem_change)
> + mode_t file_mode)
> {
> int ret = 0;
> int final_ret =0;
> @@ -273,8 +273,7 @@ int cg_chmod_recursive(struct cgroup *cgroup, mode_t
> dir_mode,
> cgroup_dbg("fts_read failed\n");
> break;
> }
> - ret = cg_chmod_file(fts, ent, dir_mode, dirm_change,
> - file_mode, filem_change);
> + ret = cg_chmod_file(fts, ent, dir_mode, file_mode);
> if (ret) {
> last_errno = errno;
> final_ret = ECGOTHER;
> diff --git a/src/tools/cgcreate.c b/src/tools/cgcreate.c
> index 7312b32..95ed3fc 100644
> --- a/src/tools/cgcreate.c
> +++ b/src/tools/cgcreate.c
> @@ -125,10 +125,8 @@ int main(int argc, char *argv[])
> int capacity = argc;
>
> /* permission variables */
> - mode_t dir_mode = 0;
> - mode_t file_mode = 0;
> - int dirm_change = 0;
> - int filem_change = 0;
> + mode_t dir_mode = -1U;
> + mode_t file_mode = -1U;
>
> /* no parametr on input */
> if (argc < 2) {
> @@ -233,11 +231,9 @@ int main(int argc, char *argv[])
> }
> break;
> case 'd':
> - dirm_change = 1;
> ret = parse_mode(optarg, &dir_mode, argv[0]);
> break;
> case 'f':
> - filem_change = 1;
> ret = parse_mode(optarg, &file_mode, argv[0]);
> break;
> default:
> @@ -310,9 +306,9 @@ int main(int argc, char *argv[])
> cgroup_free(&cgroup);
> goto err;
> }
> - if (dirm_change + filem_change > 0) {
> - ret = cg_chmod_recursive(cgroup, dir_mode, dirm_change,
> - file_mode, filem_change);
> +
> + if (dir_mode != -1U || file_mode != -1U) {
> + ret = cg_chmod_recursive(cgroup, dir_mode, file_mode);
> if (ret) {
> fprintf(stderr, "%s: can't change permission " \
> "of cgroup %s: %s\n", argv[0],
------------------------------------------------------------------------------
What Every C/C++ and Fortran developer Should Know!
Read this article and learn how Intel has extended the reach of its
next-generation tools to help Windows* and Linux* C/C++ and Fortran
developers boost performance applications - including clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
