[PATCH 3/8] _hurd_internal_post_signal: Split out inner functions

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

By having post_signal and check_pending_signal as top-level functions,
the way they communicate with the outside is made more transparent.

* hurd/hurdsig.c (_hurd_internal_post_signal): Make post_signal and
check_pending_signal top-level static helper functions.
(check_pending_signal): Fix the general poll request test being rendered
moot by earlier modifications of signo.
---
 hurd/hurdsig.c |   81 +++++++++++++++++++++++++++++--------------------------
 1 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c
index 0ea78e1..ccfea1c 100644
--- a/hurd/hurdsig.c
+++ b/hurd/hurdsig.c
@@ -426,86 +426,67 @@ abort_all_rpcs (int signo, struct machine_thread_all_state *state, int live)
   while (nthreads-- > 0)
     if (reply_ports[nthreads] != MACH_PORT_NULL)
       {
 	error_t err;
 	mach_msg_header_t head;
 	err = __mach_msg (&head, MACH_RCV_MSG|MACH_RCV_TIMEOUT, 0, sizeof head,
 			  reply_ports[nthreads],
 			  _hurd_interrupted_rpc_timeout, MACH_PORT_NULL);
 	switch (err)
 	  {
 	  case MACH_RCV_TIMED_OUT:
 	  case MACH_RCV_TOO_LARGE:
 	    break;
 
 	  default:
 	    assert_perror (err);
 	  }
       }
 }
 
 struct hurd_signal_preemptor *_hurdsig_preemptors = 0;
 sigset_t _hurdsig_preempted_set;
 
 /* XXX temporary to deal with spelling fix */
 weak_alias (_hurdsig_preemptors, _hurdsig_preempters)
 
 /* Mask of stop signals.  */
 #define STOPSIGS (sigmask (SIGTTIN) | sigmask (SIGTTOU) | \
 		  sigmask (SIGSTOP) | sigmask (SIGTSTP))
 
-/* 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);
-    }
-
 /* Actual delivery of a single signal.  Called with SS unlocked.  When
    the signal is delivered, return 1 with SS locked.  If the signal is
    being traced, return 0 with SS unlocked.   */
-int post_signal (void)
+static int
+post_signal (struct hurd_sigstate *ss,
+	     int signo, struct hurd_signal_detail *detail,
+	     int untraced, void (*reply) (void))
 {
   struct machine_thread_all_state thread_state;
   enum { stop, ignore, core, term, handle } act;
   int ss_suspended;
 
   /* Mark the signal as pending.  */
   void mark_pending (void)
     {
       __sigaddset (&ss->pending, signo);
       /* Save the details to be given to the handler when SIGNO is
 	 unblocked.  */
       ss->pending_data[signo] = *detail;
     }
 
   /* Suspend the process with SIGNO.  */
   void suspend (void)
     {
       /* Stop all other threads and mark ourselves stopped.  */
       __USEPORT (PROC,
 		 ({
 		   /* Hold the siglock while stopping other threads to be
 		      sure it is not held by another thread afterwards.  */
 		   __mutex_lock (&_hurd_siglock);
 		   __proc_dostop (port, _hurd_msgport_thread);
 		   __mutex_unlock (&_hurd_siglock);
 		   abort_all_rpcs (signo, &thread_state, 1);
 		   reply ();
 		   __proc_mark_stop (port, signo, detail->code);
 		 }));
       _hurd_stopped = 1;
@@ -839,61 +820,61 @@ int post_signal (void)
 	    _hurdsig_end_catch_fault ();
 
 	    if (! machine_get_basic_state (ss->thread, &thread_state))
 	      goto sigbomb;
 	    loc = interrupted_reply_port_location (&thread_state, 1);
 	    if (loc && *loc != MACH_PORT_NULL)
 	      /* This is the reply port for the context which called
 		 sigreturn.  Since we are abandoning that context entirely
 		 and restoring SS->context instead, destroy this port.  */
 	      __mach_port_destroy (__mach_task_self (), *loc);
 
 	    /* The thread was in sigreturn, not in any interruptible RPC.  */
 	    wait_for_reply = 0;
 
 	    assert (! __spin_lock_locked (&ss->critical_section_lock));
 	  }
 	else
 	  {
 	    int crit = __spin_lock_locked (&ss->critical_section_lock);
 
 	    wait_for_reply
 	      = (_hurdsig_abort_rpcs (ss,
 				      /* In a critical section, any RPC
 					 should be cancelled instead of
 					 restarted, regardless of
 					 SA_RESTART, so the entire
 					 "atomic" operation can be aborted
 					 as a unit.  */
 				      crit ? 0 : signo, 1,
 				      &thread_state, &state_changed,
-				      &reply)
+				      reply)
 		 != MACH_PORT_NULL);
 
 	    if (crit)
 	      {
 		/* The thread is in a critical section.  Mark the signal as
 		   pending.  When it finishes the critical section, it will
 		   check for pending signals.  */
 		mark_pending ();
 		if (state_changed)
 		  /* Some cases of interrupting an RPC must change the
 		     thread state to back out the call.  Normally this
 		     change is rolled into the warping to the handler and
 		     sigreturn, but we are not running the handler now
 		     because the thread is in a critical section.  Instead,
 		     mutate the thread right away for the RPC interruption
 		     and resume it; the RPC will return early so the
 		     critical section can end soon.  */
 		  __thread_set_state (ss->thread, MACHINE_THREAD_STATE_FLAVOR,
 				      (natural_t *) &thread_state.basic,
 				      MACHINE_THREAD_STATE_COUNT);
 		/* */
 		ss->intr_port = MACH_PORT_NULL;
 		__thread_resume (ss->thread);
 		break;
 	      }
 	  }
 
 	/* Call the machine-dependent function to set the thread up
 	   to run the signal handler, and preserve its old context.  */
 	scp = _hurd_setup_sighandler (ss, handler, signo, detail,
@@ -941,163 +922,187 @@ int post_signal (void)
 	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.  Called with
-   SS locked.  If a signal is delivered, return 1 and leave SS locked.
-   If the signal is traced, or if none can be found, return 0 with
-   SS unlocked.  */
-int check_pending_signal (void)
+/* 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)
 {
+    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;
       }
 
-    untraced = 0;
-
     if (signals_pending ())
       {
 	for (signo = 1; signo < NSIG; ++signo)
 	  if (__sigismember (&pending, signo))
 	    {
 	    deliver_pending:
 	      __sigdelset (&ss->pending, signo);
-	      *detail = ss->pending_data[signo];
+	      detail = ss->pending_data[signo];
 	      __spin_unlock (&ss->lock);
 
-	      return post_signal ();
+	      if (post_signal (ss, signo, &detail, 0, reply))
+		return ss;
+	      else
+		return NULL;
 	    }
       }
 
     /* 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 (signo == 0)
+    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 0;
-  }
+    return NULL;
+}
 
+/* 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 ())
+  if (! post_signal (ss, signo, detail, untraced, reply))
     return;
 
   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 (check_pending_signal ());
+  while (ss = check_pending_signal (ss, reply, signo == 0));
 
   /* 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:
-- 
1.7.1