lkml.org 
[lkml]   [2011]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 5/6] signal: sys_sigprocmask() needs retarget_shared_pending()
    On 04/26, Linus Torvalds wrote:
    >
    > > + sigemptyset(&new_full_set);
    > > + if (how == SIG_SETMASK)
    > > + new_full_set = current->blocked;
    > > + new_full_set.sig[0] = new_set;
    >
    > Ugh. This is just ugly.

    Agreed.

    > Could we not instead turn the whole thing into a "clear these bits"
    > and "set these bits", and get rid of the "how" entirely for the helper
    > function?
    >
    > IOW, we'd have
    >
    > switch (how) {
    > case SIG_BLOCK:
    > clear_bits = 0;
    > set_bits = new_set;
    > break;
    > case SIG_UNBLOCK:
    > clear_bits = new_set;
    > set_bits = 0;
    > break
    > case SIG_SET:
    > clear_bits = low_bits;
    > set_bits = new_set;
    > break;
    > default:
    > return -EINVAL;
    > }
    >
    > and notice how you now can do that helper function *WITHOUT* any
    > conditionals, and just make it do
    >
    > sigprocmask(&clear, &set, NULL);
    >
    > which handles all cases correctly (just "andn clear" + "or set") with
    > no if's or switch'es.
    >
    > This is why I _detest_ that idiotic "sigprocmask()" interface.

    Agreed, but...

    Yes, sigprocmask(how) is ugly, but there are sys_rt_sigprocmask() and
    sys_sigprocmask() which have to handle these SIG_* operations anyway.
    So, I think we should do:

    1. Almost all callers of sigprocmask() use SIG_SETMASK, we can
    simply change them to use set_current_blocked().

    2. Add the new helper (probably like you suggested) and convert
    other 9 callers.

    3. Unexport sigprocmask() and remove the last "*oldset" argument,
    it should be only used by sys_*sigprocmask() syscalls.

    But firstly I'd like to finish this "don't change ->blocked directly"
    conversion. And this patch changes sys_sigprocmask() so that it looks
    similar to sys_rt_sigprocmask().

    What do you think?

    If you can't accept sys_sigprocmask()->sigprocmask(), will you agree
    with

    SYSCALL_DEFINE3(sigprocmask, int, how, old_sigset_t __user *, nset,
    old_sigset_t __user *, oset)
    {
    old_sigset_t old_set, new_set;
    sigset_t new_blocked;

    old_set = current->blocked.sig[0];

    if (nset) {
    if (copy_from_user(&new_set, nset, sizeof(*nset)))
    return -EFAULT;
    new_set &= ~(sigmask(SIGKILL) | sigmask(SIGSTOP));

    new_blocked = current->blocked;

    switch (how) {
    case SIG_BLOCK:
    sigaddsetmask(&new_blocked, new_set);
    break;
    case SIG_UNBLOCK:
    sigdelsetmask(&new_blocked, new_set);
    break;
    case SIG_SETMASK:
    new_blocked.sig[0] = new_set;
    break;
    default:
    return -EINVAL;
    }

    set_current_blocked(&new_blocked);
    }

    if (oset) {
    if (copy_to_user(oset, &old_set, sizeof(*oset)))
    return -EFAULT;
    }

    return 0;
    }
    ?

    Oleg.



    \
     
     \ /
      Last update: 2011-04-27 15:01    [W:2.284 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site