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

[Jeremie Koenig <jk at jk dot fr dot eu dot org>]

* 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