On Mon, 2008-12-08 at 12:22 +0530, gowrishankar wrote: > static int pid_array_load(pid_t *pidarray, int npids, struct cgroup *cgrp) > { > - int n = 0; > + int n = 0, ret; Please declare these separately unless there's a really good reason not to. > struct cgroup_iter it; > struct task_struct *tsk; > cgroup_iter_start(cgrp, &it); > while ((tsk = cgroup_iter_next(cgrp, &it))) { > if (unlikely(n == npids)) > break; > - pidarray[n++] = task_pid_vnr(tsk); > + if ((ret = task_pid_vnr(tsk)) > 0) > + pidarray[n++] = ret; We almost never write things this way in the kernel. Too many people mistake this if() form for a coding mistake. Please separate out the assignment and condition: ret = task_pid_vnr(tsk); if (ret > 0) ... Also, 'ret' is generally used to indicate that the variable is going to be returned from the function. This one is not. > } > cgroup_iter_end(cgrp, &it); > return n; I'm not sure this is a good patch, but it needs some CodingStyle love either way. I could have sworn we had some function like task_is_in_current_active_pid_ns(), but I looked and couldn't find it. Maybe we should add one instead of open-coding this everywhere. -- Dave _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers