[PATCH 4/8] Hurd signal cleanup: refactor check_pending_signals

[Jeremie Koenig]

* hurd/hurdsig.c (check_pending_signals): Split into pending_signals,
post_pending and post_all_pending_signals.
(_hurd_internal_post_signal): Handle the distinction between poll
requests and real signals there.
---
 hurd/hurdsig.c |  134 +++++++++++++++++++++++++++++---------------------------
 1 files changed, 70 insertions(+), 64 deletions(-)

diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c
index ccfea1c..6923c87 100644
--- a/hurd/hurdsig.c
+++ b/hurd/hurdsig.c
@@ -922,191 +922,197 @@ post_signal (struct hurd_sigstate *ss,
        scp->sc_mask = ss->blocked;
        __sigorset (&ss->blocked, &ss->blocked, &ss->actions[signo].sa_mask);
 
        /* Also block SIGNO unless we're asked not to.  */
        if (! (ss->actions[signo].sa_flags & (SA_RESETHAND | SA_NODEFER)))
          __sigaddset (&ss->blocked, signo);
 
        /* Reset to SIG_DFL if requested.  SIGILL and SIGTRAP cannot
            be automatically reset when delivered; the system silently
            enforces this restriction.  */
        if (ss->actions[signo].sa_flags & SA_RESETHAND
            && signo != SIGILL && signo != SIGTRAP)
          ss->actions[signo].sa_handler = SIG_DFL;
 
        /* Start the thread running the handler (or possibly waiting for an
           RPC reply before running the handler).  */
        err = __thread_set_state (ss->thread, MACHINE_THREAD_STATE_FLAVOR,
                                  (natural_t *) &thread_state.basic,
                                  MACHINE_THREAD_STATE_COUNT);
        assert_perror (err);
        err = __thread_resume (ss->thread);
        assert_perror (err);
        thread_state.set = 0;   /* Everything we know is now wrong.  */
        break;
       }
     }
 
   return 1;
 }
 
-/* Try to find a non-blocked pending signal and deliver it, testing the
-   sigstate SS first.  Called with SS locked. If a pending signal is delivered,
-   return the corresponding sigstate, locked.  Otherwise, return NULL.  */
-static struct hurd_sigstate *
-check_pending_signal (struct hurd_sigstate *ss, void (*reply) (void), int poll)
+/* Return the set of pending signals in SS which should be delivered. */
+static sigset_t
+pending_signals (struct hurd_sigstate *ss)
+{
+  /* We don't worry about any pending signals if we are stopped, nor if
+     SS is in a critical section.  We are guaranteed to get a sig_post
+     message before any of them become deliverable: either the SIGCONT
+     signal, or a sig_post with SIGNO==0 as an explicit poll when the
+     thread finishes its critical section.  */
+  if (_hurd_stopped || __spin_lock_locked (&ss->critical_section_lock))
+    return 0;
+
+  return ss->pending & ~ss->blocked;
+}
+
+/* Post the specified pending signals in SS and return 1.  If one of
+   them is traced, abort immediately and return 0.  SS must be locked on
+   entry and will be unlocked in all cases.  */
+static int
+post_pending (struct hurd_sigstate *ss, sigset_t pending, void (*reply) (void))
 {
     int signo;
     struct hurd_signal_detail detail;
-    sigset_t pending;
 
-    /* Return nonzero if SS has any signals pending we should worry about.
-       We don't worry about any pending signals if we are stopped, nor if
-       SS is in a critical section.  We are guaranteed to get a sig_post
-       message before any of them become deliverable: either the SIGCONT
-       signal, or a sig_post with SIGNO==0 as an explicit poll when the
-       thread finishes its critical section.  */
-    inline int signals_pending (void)
-      {
-       if (_hurd_stopped || __spin_lock_locked (&ss->critical_section_lock))
-         return 0;
-       return pending = ss->pending & ~ss->blocked;
-      }
-
-    if (signals_pending ())
-      {
        for (signo = 1; signo < NSIG; ++signo)
          if (__sigismember (&pending, signo))
            {
-           deliver_pending:
              __sigdelset (&ss->pending, signo);
              detail = ss->pending_data[signo];
              __spin_unlock (&ss->lock);
 
-             if (post_signal (ss, signo, &detail, 0, reply))
-               return ss;
-             else
-               return NULL;
+             /* Will reacquire the lock, except if the signal is traced.  */
+             if (! post_signal (ss, signo, &detail, 0, reply))
+               return 0;
            }
-      }
 
-    /* No pending signals left undelivered for this thread.
-       If we were sent signal 0, we need to check for pending
-       signals for all threads.  */
-    if (poll)
-      {
-       __spin_unlock (&ss->lock);
-       __mutex_lock (&_hurd_siglock);
-       for (ss = _hurd_sigstates; ss != NULL; ss = ss->next)
-         {
-           __spin_lock (&ss->lock);
-           for (signo = 1; signo < NSIG; ++signo)
-             if (__sigismember (&ss->pending, signo)
-                 && (!__sigismember (&ss->blocked, signo)
-                     /* We "deliver" immediately pending blocked signals whose
-                        action might be to ignore, so that if ignored they are
-                        dropped right away.  */
-                     || ss->actions[signo].sa_handler == SIG_IGN
-                     || ss->actions[signo].sa_handler == SIG_DFL))
-               {
-                 mutex_unlock (&_hurd_siglock);
-                 goto deliver_pending;
-               }
-           __spin_unlock (&ss->lock);
-         }
-       __mutex_unlock (&_hurd_siglock);
-      }
-    else
-      {
        /* No more signals pending; SS->lock is still locked.
           Wake up any sigsuspend call that is blocking SS->thread.  */
        if (ss->suspended != MACH_PORT_NULL)
          {
            /* There is a sigsuspend waiting.  Tell it to wake up.  */
            error_t err;
            mach_msg_header_t msg;
            msg.msgh_bits = MACH_MSGH_BITS (MACH_MSG_TYPE_MAKE_SEND, 0);
            msg.msgh_remote_port = ss->suspended;
            msg.msgh_local_port = MACH_PORT_NULL;
            /* These values do not matter.  */
            msg.msgh_id = 8675309; /* Jenny, Jenny.  */
            ss->suspended = MACH_PORT_NULL;
            err = __mach_msg (&msg, MACH_SEND_MSG, sizeof msg, 0,
                              MACH_PORT_NULL, MACH_MSG_TIMEOUT_NONE,
                              MACH_PORT_NULL);
            assert_perror (err);
          }
        __spin_unlock (&ss->lock);
-      }
 
-    return NULL;
+    return 1;
+}
+
+/* Post all the pending signals of all threads and return 1.  If a traced
+   signal is encountered, abort immediately and return 0.  */
+static int
+post_all_pending_signals (void (*reply) (void))
+{
+  struct hurd_sigstate *ss;
+  sigset_t pending;
+
+  for (;;)
+    {
+      __mutex_lock (&_hurd_siglock);
+      for (ss = _hurd_sigstates; ss != NULL; ss = ss->next)
+        {
+         __spin_lock (&ss->lock);
+
+         pending = pending_signals (ss);
+         if (pending)
+           /* post_pending() below will unlock SS. */
+           break;
+
+         __spin_unlock (&ss->lock);
+       }
+      __mutex_unlock (&_hurd_siglock);
+
+      if (! pending)
+       return 1;
+      if (! post_pending (ss, pending, reply))
+       return 0;
+    }
 }
 
 /* Deliver a signal.  SS is not locked.  */
 void
 _hurd_internal_post_signal (struct hurd_sigstate *ss,
                            int signo, struct hurd_signal_detail *detail,
                            mach_port_t reply_port,
                            mach_msg_type_name_t reply_port_type,
                            int untraced)
 {
   /* Reply to this sig_post message.  */
   __typeof (__msg_sig_post_reply) *reply_rpc
     = (untraced ? __msg_sig_post_untraced_reply : __msg_sig_post_reply);
   void reply (void)
     {
       error_t err;
       if (reply_port == MACH_PORT_NULL)
        return;
       err = (*reply_rpc) (reply_port, reply_port_type, 0);
       reply_port = MACH_PORT_NULL;
       if (err != MACH_SEND_INVALID_DEST) /* Ignore dead reply port.  */
        assert_perror (err);
     }
 
   if (! post_signal (ss, signo, detail, untraced, reply))
     return;
 
+  /* The signal was neither fatal nor traced.  We still hold SS->lock.  */
   if (signo != 0)
     {
       /* The signal has either been ignored or is now being handled.  We can
         consider it delivered and reply to the killer.  */
       reply ();
-    }
 
-  /* We get here unless the signal was fatal.  We still hold SS->lock.
-     Check for pending signals, and loop to post them.  */
-  while (ss = check_pending_signal (ss, reply, signo == 0));
+      /* Post any pending signals for this thread.  */
+      if (! post_pending (ss, pending_signals (ss), reply))
+       return;
+    }
+  else
+    {
+      /* We need to check for pending signals for all threads.  */
+      __spin_unlock (&ss->lock);
+      if (! post_all_pending_signals (reply))
+       return;
 
-  /* All pending signals delivered to all threads.
-     Now we can send the reply message even for signal 0.  */
-  reply ();
+      /* All pending signals delivered to all threads.
+        Now we can send the reply message even for signal 0.  */
+      reply ();
+    }
 }
 
 /* Decide whether REFPORT enables the sender to send us a SIGNO signal.
    Returns zero if so, otherwise the error code to return to the sender.  */
 
 static error_t
 signal_allowed (int signo, mach_port_t refport)
 {
   if (signo < 0 || signo >= NSIG)
     return EINVAL;
 
   if (refport == __mach_task_self ())
     /* Can send any signal.  */
     goto win;
 
   /* Avoid needing to check for this below.  */
   if (refport == MACH_PORT_NULL)
     return EPERM;
 
   switch (signo)
     {
     case SIGINT:
     case SIGQUIT:
     case SIGTSTP:
     case SIGHUP:
     case SIGINFO:
     case SIGTTIN:
     case SIGTTOU:
     case SIGWINCH:
       /* Job control signals can be sent by the controlling terminal.  */
-- 
1.7.1