RFA: lin-lwp cleanup

RFA: lin-lwp cleanup

[Daniel Jacobowitz]

This patch doesn't do anything particularly important.  I just moved some
code from stop_wait_callback out to a new function.  I thought I'd need it
for my next patch; I turned out not to, but it's still cleaner this way.

Also fixes the two small problems I asked Jeff about earlier today - an
extra call to lin_lwp_thread_alive and a missing delete_thread.

Is this OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-08-26  Daniel Jacobowitz  <drow@mvista.com>

	* lin-lwp.c (wait_lwp): New function, copied from
	stop_wait_callback.  Clean up.
	(stop_wait_callback): Use wait_lwp.

Index: lin-lwp.c
===================================================================
RCS file: /cvs/src/src/gdb/lin-lwp.c,v
retrieving revision 1.49
diff -u -p -r1.49 lin-lwp.c
--- lin-lwp.c	17 Aug 2003 18:52:59 -0000	1.49
+++ lin-lwp.c	26 Aug 2003 19:26:43 -0000
@@ -585,6 +585,77 @@ kill_lwp (int lwpid, int signo)
   return kill (lwpid, signo);
 }
 
+/* Wait for LP to stop.  Returns the wait status, or 0 if the LWP has
+   exited.  */
+
+static int
+wait_lwp (struct lwp_info *lp)
+{
+  pid_t pid;
+  int status;
+  int thread_dead = 0;
+
+  gdb_assert (!lp->stopped);
+  gdb_assert (lp->status == 0);
+
+  pid = waitpid (GET_LWP (lp->ptid), &status, 0);
+  if (pid == -1 && errno == ECHILD)
+    {
+      pid = waitpid (GET_LWP (lp->ptid), &status, __WCLONE);
+      if (pid == -1 && errno == ECHILD)
+	{
+	  /* The thread has previously exited.  We need to delete it now
+	     because in the case of NPTL threads, there won't be an
+	     exit event unless it is the main thread.  */
+	  thread_dead = 1;
+	  if (debug_lin_lwp)
+	    fprintf_unfiltered (gdb_stdlog, "WL: %s vanished.\n",
+				target_pid_to_str (lp->ptid));
+	}
+    }
+
+  if (!thread_dead)
+    {
+      gdb_assert (pid == GET_LWP (lp->ptid));
+
+      if (debug_lin_lwp)
+	{
+	  fprintf_unfiltered (gdb_stdlog,
+			      "WL: waitpid %s received %s\n",
+			      target_pid_to_str (lp->ptid),
+			      status_to_str (status));
+	}
+    }
+
+  /* Check if the thread has exited.  */
+  if (WIFEXITED (status) || WIFSIGNALED (status))
+    {
+      thread_dead = 1;
+      if (debug_lin_lwp)
+	fprintf_unfiltered (gdb_stdlog, "WL: %s exited.\n",
+			    target_pid_to_str (lp->ptid));
+    }
+
+  if (thread_dead)
+    {
+      if (in_thread_list (lp->ptid))
+	{
+	  /* Core GDB cannot deal with us deleting the current thread.  */
+	  if (!ptid_equal (lp->ptid, inferior_ptid))
+	    delete_thread (lp->ptid);
+	  printf_unfiltered ("[%s exited]\n",
+			     target_pid_to_str (lp->ptid));
+	}
+
+      delete_lwp (lp->ptid);
+      return 0;
+    }
+
+  gdb_assert (WIFSTOPPED (status));
+
+  return status;
+}
+
 /* Send a SIGSTOP to LP.  */
 
 static int
@@ -627,86 +698,11 @@ stop_wait_callback (struct lwp_info *lp,
 
   if (!lp->stopped && lp->signalled)
     {
-      pid_t pid;
       int status;
 
-      gdb_assert (lp->status == 0);
-
-      pid = waitpid (GET_LWP (lp->ptid), &status, 0);
-      if (pid == -1 && errno == ECHILD)
-	{
-	  pid = waitpid (GET_LWP (lp->ptid), &status, __WCLONE);
-	  if (pid == -1 && errno == ECHILD)
-	    {
-	      /* The thread has previously exited.  We need to delete it now
-	         because in the case of nptl threads, there won't be an
-	         exit event unless it is the main thread.  */
-	      if (debug_lin_lwp)
-		fprintf_unfiltered (gdb_stdlog,
-				    "SWC: %s exited.\n",
-				    target_pid_to_str (lp->ptid));
-	      delete_lwp (lp->ptid);
-	      return 0;
-	    }
-	}
-
-      gdb_assert (pid == GET_LWP (lp->ptid));
-
-      if (debug_lin_lwp)
-	{
-	  fprintf_unfiltered (gdb_stdlog,
-			      "SWC: waitpid %s received %s\n",
-			      target_pid_to_str (lp->ptid),
-			      status_to_str (status));
-	}
-
-      /* Check if the thread has exited.  */
-      if (WIFEXITED (status) || WIFSIGNALED (status))
-	{
-	  gdb_assert (num_lwps > 1);
-
-	  if (in_thread_list (lp->ptid))
-	    {
-	      /* Core GDB cannot deal with us deleting the current
-	         thread.  */
-	      if (!ptid_equal (lp->ptid, inferior_ptid))
-		delete_thread (lp->ptid);
-	      printf_unfiltered ("[%s exited]\n",
-				 target_pid_to_str (lp->ptid));
-	    }
-	  if (debug_lin_lwp)
-	    fprintf_unfiltered (gdb_stdlog,
-				"SWC: %s exited.\n",
-				target_pid_to_str (lp->ptid));
-
-	  delete_lwp (lp->ptid);
-	  return 0;
-	}
-
-      /* Check if the current LWP has previously exited.  For nptl threads,
-         there is no exit signal issued for LWPs that are not the
-         main thread so we should check whenever the thread is stopped.  */
-      if (!lin_lwp_thread_alive (lp->ptid))
-	{
-	  if (in_thread_list (lp->ptid))
-	    {
-	      /* Core GDB cannot deal with us deleting the current
-	         thread.  */
-	      if (!ptid_equal (lp->ptid, inferior_ptid))
-		delete_thread (lp->ptid);
-	      printf_unfiltered ("[%s exited]\n",
-				 target_pid_to_str (lp->ptid));
-	    }
-	  if (debug_lin_lwp)
-	    fprintf_unfiltered (gdb_stdlog,
-				"SWC: %s already exited.\n",
-				target_pid_to_str (lp->ptid));
-
-	  delete_lwp (lp->ptid);
-	  return 0;
-	}
-
-      gdb_assert (WIFSTOPPED (status));
+      status = wait_lwp (lp);
+      if (status == 0)
+	return 0;
 
       /* Ignore any signals in FLUSH_MASK.  */
       if (flush_mask && sigismember (flush_mask, WSTOPSIG (status)))

Re: RFA: lin-lwp cleanup

[Michael Snyder]

Daniel Jacobowitz wrote:
> This patch doesn't do anything particularly important.  I just moved some
> code from stop_wait_callback out to a new function.  I thought I'd need it
> for my next patch; I turned out not to, but it's still cleaner this way.
> 
> Also fixes the two small problems I asked Jeff about earlier today - an
> extra call to lin_lwp_thread_alive and a missing delete_thread.
> 
> Is this OK?
> 

There's a bit more here than code movement -- the new code is not
identical to the old, even allowing for the jjohnstn changes.

If you'll say a word or two about the differences, I expect I'll approve 
them.

Re: RFA: lin-lwp cleanup

[Daniel Jacobowitz]

On Tue, Aug 26, 2003 at 07:18:55PM -0700, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> >This patch doesn't do anything particularly important.  I just moved some
> >code from stop_wait_callback out to a new function.  I thought I'd need it
> >for my next patch; I turned out not to, but it's still cleaner this way.
> >
> >Also fixes the two small problems I asked Jeff about earlier today - an
> >extra call to lin_lwp_thread_alive and a missing delete_thread.
> >
> >Is this OK?
> >
> 
> There's a bit more here than code movement -- the new code is not
> identical to the old, even allowing for the jjohnstn changes.
> 
> If you'll say a word or two about the differences, I expect I'll approve 
> them.

The differences are exactly those two.  There were two copies of the
code which called delete_lwp, and one of them was missing
delete_thread; so I collapsed them together.  And there was a block
which checked lin_lwp_thread_alive, now gone.

Oh, I changed the text of the first error message from "exited" to
"vanished" so that we could tell from the logs which case was used.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA: lin-lwp cleanup

[Michael Snyder]

Daniel Jacobowitz wrote:
> On Tue, Aug 26, 2003 at 07:18:55PM -0700, Michael Snyder wrote:
> 
>>Daniel Jacobowitz wrote:
>>
>>>This patch doesn't do anything particularly important.  I just moved some
>>>code from stop_wait_callback out to a new function.  I thought I'd need it
>>>for my next patch; I turned out not to, but it's still cleaner this way.
>>>
>>>Also fixes the two small problems I asked Jeff about earlier today - an
>>>extra call to lin_lwp_thread_alive and a missing delete_thread.
>>>
>>>Is this OK?
>>>
>>
>>There's a bit more here than code movement -- the new code is not
>>identical to the old, even allowing for the jjohnstn changes.
>>
>>If you'll say a word or two about the differences, I expect I'll approve 
>>them.
> 
> 
> The differences are exactly those two.  There were two copies of the
> code which called delete_lwp, and one of them was missing
> delete_thread; so I collapsed them together.  And there was a block
> which checked lin_lwp_thread_alive, now gone.
> 
> Oh, I changed the text of the first error message from "exited" to
> "vanished" so that we could tell from the logs which case was used.
> 

OK then.  Looks fine to me.

Re: RFA: lin-lwp cleanup

[Daniel Jacobowitz]

On Wed, Aug 27, 2003 at 10:38:33AM -0700, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> >On Tue, Aug 26, 2003 at 07:18:55PM -0700, Michael Snyder wrote:
> >
> >>Daniel Jacobowitz wrote:
> >>
> >>>This patch doesn't do anything particularly important.  I just moved some
> >>>code from stop_wait_callback out to a new function.  I thought I'd need 
> >>>it
> >>>for my next patch; I turned out not to, but it's still cleaner this way.
> >>>
> >>>Also fixes the two small problems I asked Jeff about earlier today - an
> >>>extra call to lin_lwp_thread_alive and a missing delete_thread.
> >>>
> >>>Is this OK?
> >>>
> >>
> >>There's a bit more here than code movement -- the new code is not
> >>identical to the old, even allowing for the jjohnstn changes.
> >>
> >>If you'll say a word or two about the differences, I expect I'll approve 
> >>them.
> >
> >
> >The differences are exactly those two.  There were two copies of the
> >code which called delete_lwp, and one of them was missing
> >delete_thread; so I collapsed them together.  And there was a block
> >which checked lin_lwp_thread_alive, now gone.
> >
> >Oh, I changed the text of the first error message from "exited" to
> >"vanished" so that we could tell from the logs which case was used.
> >
> 
> OK then.  Looks fine to me.

Thanks, checked in then.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer