Christoph Hellwig <[EMAIL PROTECTED]> writes:
> On Sat, Dec 16, 2006 at 11:05:10PM +0300, Oleg Nesterov wrote:
>> static int kill_something_info(int sig, struct siginfo *info, int pid)
>> {
>> int ret;
>> +
>> + rcu_read_lock();
>> + if (pid > 0) {
>> + ret = kill_pid_info(sig, info, find_pid(pid));
>> + } else if (pid == -1) {
>> + struct task_struct *p;
>> + int found = 0;
>> +
>> + ret = 0;
>> + read_lock(&tasklist_lock);
>> + for_each_process(p)
>> + if (!is_init(p) && p != current->group_leader) {
>> + int err = group_send_sig_info(sig, info, p);
>> + if (err != -EPERM)
>> + ret = err;
>> + found = 1;
>> + }
>> + read_unlock(&tasklist_lock);
>> + if (!found)
>> + ret = -ESRCH;
>
> This branch should probably be factored out into a helper of it's own:
The proper name would be something like kill_all_info(). As we are
talking about the group of all processes.
I am sitting here wondering why we bother to ignore init, as init
is protected from all signals it doesn't explicitly setup a signal
handler for. It is probably worth taking a quick look at the common
shutdown scripts and sysv init and see if anything actually cares if
we simply remove the is_init check.
The only two signals I know that are commonly handled this way
are kill(-1, SIGTERM) and kill(-1, SIGKILL);
And a very quick look at sysvinit-2.86 shows that it doesn't setup a
handler for SIGTERM. So I believe we can delete we can delete
the is_init check entirely without changing anything and with a less
surprising if anyone ever cares.
>> + } else {
>> + struct pid *grp = task_pgrp(current);
>> + if (pid != 0)
>> + grp = find_pid(-pid);
>> + ret = kill_pgrp_info(sig, info, grp);
>> + }
>
> This also looks rather unreadable, an
>
> } else if (pid) {
> ret = kill_pgrp_info(sig, info, find_pid(-pid));
> } else {
> ret = kill_pgrp_info(sig, info, task_pgrp(current));
> }
>
> might be slightly more code, but also a lot more readable.
And that part is basically what we have now, just reshuffled.
Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
