ebiederm@xxxxxxxxxxxx (Eric W. Biederman) writes:
> Using pid_task(find_vpid(N), PIDTYPE_TGID) guarantees that if a task
> is found it is at that moment the thread group leader. Which removes
> the need for the follow on test has_group_leader_pid.
>
> I have reorganized the rest of the code in lookup_task for clarity,
> and created a common return for most of the code.
Sorry, it's way harder to read than the very explicit exits which were
there before.
> The special case for clock_gettime with "pid == gettid" is not my
> favorite. I strongly suspect it isn't used as gettid is such a pain,
> and passing 0 is much easier. Still it is easier to keep this special
> case than to do the reasarch that will show it isn't used.
It might be not your favorite, but when I refactored the code I learned
the hard way that one of the test suites has assumptions that
clock_gettime(PROCESS) works from any task of a group and not just for
the group leader. Sure we could fix the test suite, but test code tends
to be copied ...
> /*
> * Functions for validating access to tasks.
> */
> -static struct task_struct *lookup_task(const pid_t pid, bool thread,
> +static struct task_struct *lookup_task(const pid_t which_pid, bool thread,
> bool gettime)
> {
> struct task_struct *p;
> + struct pid *pid;
>
> /*
> * If the encoded PID is 0, then the timer is targeted at current
> * or the process to which current belongs.
> */
> - if (!pid)
> + if (!which_pid)
> return thread ? current : current->group_leader;
>
> - p = find_task_by_vpid(pid);
> - if (!p)
> - return p;
> -
> - if (thread)
> - return same_thread_group(p, current) ? p : NULL;
> -
> - if (gettime) {
> + pid = find_vpid(which_pid);
> + if (thread) {
> + p = pid_task(pid, PIDTYPE_PID);
> + if (p && !same_thread_group(p, current))
> + p = NULL;
> + } else {
> /*
> * For clock_gettime(PROCESS) the task does not need to be
> * the actual group leader. tsk->sighand gives
> @@ -76,13 +75,13 @@ static struct task_struct *lookup_task(const pid_t pid, bool thread,
> * reference on it and store the task pointer until the
> * timer is destroyed.
Btw, this comment is wrong since
55e8c8eb2c7b ("posix-cpu-timers: Store a reference to a pid not a task")
Thanks,
tglx
