On Sat, Oct 23, 2021 at 05:47:58PM +0200, Mark Kettenis wrote:
> > Date: Sat, 23 Oct 2021 17:29:36 +0200
> > From: Claudio Jeker <[email protected]>
> >
> > 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 code does the check first. It just sets the error before doing the
pending signal check. If a signal is pending the error is reset to 0 and
the function returns. Else it will break out of the loop with the error
and not hit the tsleep.
> 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;
> >
> >
--
:wq Claudio
