[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebsd-hackers
Subject:    Re: [PATCH] Catch errors with sigaddset(3) in sigaddset (sighold)
From:       Garrett Cooper <yanegomi () gmail ! com>
Date:       2010-07-20 19:00:44
Message-ID: AANLkTin_zxwyPPbnQWSCMtix9iHFo7nVRg4g1-Z_UTX1 () mail ! gmail ! com
[Download RAW message or body]

On Mon, Jul 19, 2010 at 5:09 AM, Kostik Belousov <kostikbel@gmail.com> wrote:
> On Sun, Jul 18, 2010 at 10:46:25PM -0700, Garrett Cooper wrote:
>>     sighold(3) doesn't determine whether or not the signal added is
>> valid today (and sigprocmask doesn't verify that either). This fixes
>> that.
>> Thanks,
>> -Garrett
>>
>> Index: sigcompat.c
>> ===================================================================
>> --- sigcompat.c       (revision 210226)
>> +++ sigcompat.c       (working copy)
>> @@ -131,7 +131,8 @@
>>       sigset_t set;
>>
>>       sigemptyset(&set);
>> -     sigaddset(&set, sig);
>> +     if (sigaddset(&set, sig) == -1)
>> +             return (-1);
>>       return (_sigprocmask(SIG_BLOCK, &set, NULL));
>>  }
>
> I added checks for failures of sig{add,del}set to the sigcompat.c,
> and unified style to not assign intermediate error to local variable.
>
> This is what I am going to commit shortly.
>
> diff --git a/lib/libc/compat-43/sigcompat.c b/lib/libc/compat-43/sigcompat.c
> index 6841eeb..199846f 100644
> --- a/lib/libc/compat-43/sigcompat.c
> +++ b/lib/libc/compat-43/sigcompat.c
> @@ -112,16 +112,11 @@ int
>  xsi_sigpause(int sig)
>  {
>        sigset_t set;
> -       int error;
>
> -       if (!_SIG_VALID(sig)) {
> -               errno = EINVAL;
> +       if (_sigprocmask(SIG_BLOCK, NULL, &set) == -1)
> +               return (-1);
> +       if (sigdelset(&set, sig) == -1)
>                return (-1);
> -       }
> -       error = _sigprocmask(SIG_BLOCK, NULL, &set);
> -       if (error != 0)
> -               return (error);
> -       sigdelset(&set, sig);
>        return (_sigsuspend(&set));
>  }
>
> @@ -131,7 +126,8 @@ sighold(int sig)
>        sigset_t set;
>
>        sigemptyset(&set);
> -       sigaddset(&set, sig);
> +       if (sigaddset(&set, sig) == -1)
> +               return (-1);
>        return (_sigprocmask(SIG_BLOCK, &set, NULL));
>  }
>
> @@ -151,7 +147,8 @@ sigrelse(int sig)
>        sigset_t set;
>
>        sigemptyset(&set);
> -       sigaddset(&set, sig);
> +       if (sigaddset(&set, sig) == -1)
> +               return (-1);
>        return (_sigprocmask(SIG_UNBLOCK, &set, NULL));
>  }
>
> @@ -160,35 +157,30 @@ void
>  {
>        sigset_t set, pset;
>        struct sigaction sa, psa;
> -       int error;
>
>        sigemptyset(&set);
> -       sigaddset(&set, sig);
> -       error = _sigprocmask(SIG_BLOCK, NULL, &pset);
> -       if (error == -1)
> +       if (sigaddset(&set, sig) == -1)
> +               return (SIG_ERR);
> +       if (_sigprocmask(SIG_BLOCK, NULL, &pset) == -1)
>                return (SIG_ERR);
>        if ((__sighandler_t *)disp == SIG_HOLD) {
> -               error = _sigprocmask(SIG_BLOCK, &set, &pset);
> -               if (error == -1)
> +               if (_sigprocmask(SIG_BLOCK, &set, &pset) == -1)
>                        return (SIG_ERR);
>                if (sigismember(&pset, sig))
>                        return (SIG_HOLD);
>                else {
> -                       error = _sigaction(sig, NULL, &psa);
> -                       if (error == -1)
> +                       if (_sigaction(sig, NULL, &psa) == -1)
>                                return (SIG_ERR);
>                        return (psa.sa_handler);
>                }
>        } else {
> -               error = _sigprocmask(SIG_UNBLOCK, &set, &pset);
> -               if (error == -1)
> +               if (_sigprocmask(SIG_UNBLOCK, &set, &pset) == -1)
>                        return (SIG_ERR);
>        }
>
>        bzero(&sa, sizeof(sa));
>        sa.sa_handler = disp;
> -       error = _sigaction(sig, &sa, &psa);
> -       if (error == -1)
> +       if (_sigaction(sig, &sa, &psa) == -1)
>                return (SIG_ERR);
>        if (sigismember(&pset, sig))
>                return (SIG_HOLD);

    The results when I ran the tests manually outside of the shell
script were ok. I need to track down why they're failing in the script
itself.
Thanks,
-Garrett
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"

[prev in list] [next in list] [prev in thread] [next in thread]