> Date: Sat, 23 Oct 2021 17:29:36 +0200
> From: Claudio Jeker <cje...@diehard.n-r-g.com>
>
> The sys___thrsigdivert code can be simplified a bit. It is possible to
> set the error before the loop and then have the loop exit after polling
> for pending signals. IMO the results looks nicer than what we have now.
>
> OK?
That does not look right:
The function sigtimedwait() behaves the same as sigwaitinfo() except
that if none of the signals specified by set are pending,
sigtimedwait() waits for the time interval specified in the timespec
structure referenced by timeout. If the timespec structure pointed
to by timeout is zero-valued and if none of the signals specified by
set are pending, then sigtimedwait() returns immediately with an
error. If timeout is the NULL pointer, the behaviour is unspecified.
So you need to check for signals first.
The current code looks fine to me.
> Index: kern/kern_sig.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.286
> diff -u -p -r1.286 kern_sig.c
> --- kern/kern_sig.c 23 Oct 2021 14:56:55 -0000 1.286
> +++ kern/kern_sig.c 23 Oct 2021 15:15:21 -0000
> @@ -1761,7 +1761,6 @@ sys___thrsigdivert(struct proc *p, void
> sigset_t mask = SCARG(uap, sigmask) &~ sigcantmask;
> siginfo_t si;
> uint64_t nsecs = INFSLP;
> - int timeinvalid = 0;
> int error = 0;
>
> memset(&si, 0, sizeof(si));
> @@ -1774,12 +1773,21 @@ sys___thrsigdivert(struct proc *p, void
> if (KTRPOINT(p, KTR_STRUCT))
> ktrreltimespec(p, &ts);
> #endif
> - if (!timespecisvalid(&ts))
> - timeinvalid = 1;
> - else
> + if (timespecisvalid(&ts)) {
> nsecs = TIMESPEC_TO_NSEC(&ts);
> + } else {
> + /*
> + * per-POSIX, delay this error until
> + * after checking for signals
> + */
> + error = EINVAL;
> + }
> }
>
> + /* per-POSIX, return immediatly if timeout is zero-valued */
> + if (nsecs == 0)
> + error = EAGAIN;
> +
> dosigsuspend(p, p->p_sigmask &~ mask);
> for (;;) {
> si.si_signo = cursig(p);
> @@ -1791,13 +1799,6 @@ sys___thrsigdivert(struct proc *p, void
> break;
> }
> }
> -
> - /* per-POSIX, delay this error until after the above */
> - if (timeinvalid)
> - error = EINVAL;
> - /* per-POSIX, return immediatly if timeout is zero-valued */
> - if (nsecs == 0)
> - error = EAGAIN;
>
> if (error != 0)
> break;
>
>
