[lkml]   [2011]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH v2 5/6] signal: sys_sigprocmask() needs retarget_shared_pending()
    > +               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.

    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

    IOW, we'd have

    switch (how) {
    case SIG_BLOCK:
    clear_bits = 0;
    set_bits = new_set;
    case SIG_UNBLOCK:
    clear_bits = new_set;
    set_bits = 0;
    case SIG_SET:
    clear_bits = low_bits;
    set_bits = new_set;
    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. That
    "how" parameter is the invention of somebody who didn't understand
    sets. It's stupid. There is no reason to have multiple different set
    operations, when in practice all anybody ever wants is the "clear
    these bits and set those other bits" - an operation that is not only
    more generic than the idiotic "how", but is _faster_ too, because it
    involves no conditionals.

    So I realize that we cannot get away from the broken user interface,
    but I do not believe that that means that our _internal_ helper
    functions should use that idiotic and broken interface!

    I had basically this same comment earlier when you did something
    similarly mindless for another case.

    So basic rule should be: if you ever pass "how" to any helper
    functions, it's broken.


     \ /
      Last update: 2011-04-26 23:45    [W:0.022 / U:75.460 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site