On Wed, Jan 07, 2009 at 01:47:16PM +0530, Balbir Singh wrote:
> * Sudhir Kumar <[email protected]> [2009-01-02 17:02:04]:
>
> > This patch makes the function check_task() capable of checking if a pid
> > other than the calling process is attached to a particlular group. Earlier
> > it was checking for the calling process only.
> >
> > Signed-off-by: Sudhir Kumar <[email protected]>
> >
> > ---
> > tests/functions.c | 12 ++++++++----
> > tests/libcgrouptest.h | 2 +-
> > 2 files changed, 9 insertions(+), 5 deletions(-)
> >
> > Index: trunk/tests/functions.c
> > ===================================================================
> > --- trunk.orig/tests/functions.c
> > +++ trunk/tests/functions.c
> > @@ -83,11 +83,11 @@ void test_cgroup_attach_task(int retcode
> > build_path(tasksfile, mountpoint,
> > group1, "tasks");
> >
> > - if (check_task(tasksfile)) {
> > + if (check_task(tasksfile, 0)) {
> > if (fs_mounted == 2) { /* multiple mounts */
> > build_path(tasksfile2, mountpoint2,
> > group2, "tasks");
> > - if (check_task(tasksfile2)) {
> > + if (check_task(tasksfile2, 0)) {
> > message(i, PASS, "attach_task()",
> > retval, info[TASKINGRP]);
> > } else {
> > @@ -495,7 +495,7 @@ int check_fsmounted(int multimnt)
> > return 1;
> > }
> >
> > -int check_task(char *tasksfile)
> > +int check_task(char *tasksfile, pid_t pid)
> > {
> > FILE *file;
> > pid_t curr_tid, tid;
> > @@ -508,7 +508,11 @@ int check_task(char *tasksfile)
> > exit(1);
> > }
> >
> > - curr_tid = cgrouptest_gettid();
> > + if (pid)
> > + curr_tid = pid;
> > + else
> > + curr_tid = cgrouptest_gettid();
> > +
>
> This is a bit confusing and I think I understand your previous patch
> as well now, pid is a boolean + pid value. If not set, it means
> default tid, else take the passed pid. Remember pid 0 is valid, all
> though it is not explicitly used from user space or exported to it.
>
> I don't like this style of assuming that pid "0" is invalid or means
> current tid, I would rather pass it explictly from the callers.
>
Balbir, pid 0 in terms of cgroups implies the current process. I am
quite fine with this. Also, if you notice, he does a gettid() to get the
pid of the current process to check if it is in the tasks file or not.
which reminds me, it might be a nice optimization in the library as
well. If we want to move the current task, we use 0 instead of doing a
gettid()
> > while (!feof(file)) {
> > fscanf(file, "%u", &tid);
> > if (tid == curr_tid) {
> > Index: trunk/tests/libcgrouptest.h
> > ===================================================================
> > --- trunk.orig/tests/libcgrouptest.h
> > +++ trunk/tests/libcgrouptest.h
> > @@ -138,7 +138,7 @@ struct cgroup *new_cgroup(char *group, c
> > char *control_file, int value_type, struct cntl_val_t cval,
> > struct uid_gid_t ids, int i);
> > int check_fsmounted(int multimnt);
> > -int check_task(char *tasksfile);
> > +int check_task(char *tasksfile, pid_t pid);
> > /* function to print messages in better format */
> > void message(int num, int pass, const char *api,
> > int ret, char *extra);
> >
> >
> >
>
> --
> Balbir
--
regards,
Dhaval
------------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It is the best place to buy or sell services for
just about anything Open Source.
http://p.sf.net/sfu/Xq1LFB
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
