Re: [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2669 bytes --]

On Friday 11 July 2008 12:16:15, Pedro Alves wrote:
> On Monday 07 July 2008 19:59:30, Daniel Jacobowitz wrote:

> > > @@ -877,18 +1057,10 @@ thread_apply_command (char *tidlist, int
> > >    if (*cmd == '\000')
> > >      error (_("Please specify a command following the thread ID
> > > list"));
> > >
> > > -  current_ptid = inferior_ptid;
> > > -
> > > -  if (!is_running (inferior_ptid))
> > > -    saved_frame_id = get_frame_id (get_selected_frame (NULL));
> > > -  else
> > > -    saved_frame_id = null_frame_id;
> > > -  old_chain = make_cleanup_restore_current_thread (inferior_ptid,
> > > saved_frame_id); -
> > >    /* Save a copy of the command in case it is clobbered by
> > >       execute_command */
> > >    saved_cmd = xstrdup (cmd);
> > > -  saved_cmd_cleanup_chain = make_cleanup (xfree, (void *) saved_cmd);
> > > +  old_chain = make_cleanup (xfree, saved_cmd);
> > >    while (tidlist < cmd)
> > >      {
> > >        struct thread_info *tp;
> > > @@ -926,26 +1098,24 @@ thread_apply_command (char *tidlist, int
> > >  	    warning (_("Thread %d has terminated."), start);
> > >  	  else
> > >  	    {
> > > +	      make_cleanup_restore_current_thread ();
> > > +
> > >  	      if (non_stop)
> > >  		context_switch_to (tp->ptid);
> > >  	      else
> > >  		switch_to_thread (tp->ptid);
> > > +
> > >  	      printf_filtered (_("\nThread %d (%s):\n"), tp->num,
> > >  			       target_tid_to_str (inferior_ptid));
> > >  	      execute_command (cmd, from_tty);
> > > -	      strcpy (cmd, saved_cmd);	/* Restore exact command used
> > > previously */ +
> > > +	      /* Restore exact command used previously.  */
> > > +	      strcpy (cmd, saved_cmd);
> > >  	    }
> > >  	}
> > >      }
> > >
> > > -  if (!ptid_equal (current_ptid, inferior_ptid))
> > > -    thread_has_changed = 1;
> > > -
> > > -  do_cleanups (saved_cmd_cleanup_chain);
> > >    do_cleanups (old_chain);
> > > -  /* Print stack frame only if we changed thread.  */
> > > -  if (thread_has_changed)
> > > -    print_stack_frame (get_current_frame (), 1, SRC_LINE);
> > >  }
> > >
> > >  /* Switch to the specified thread.  Will dispatch off to
> > > thread_apply_command
> >
> > You've moved creation of the cleanup into the loop, but not moved the
> > call to do_cleanups.  Do we ever need more than one cleanup for a
> > thread apply command?
>
> Ooops, sorry, I missed this, and committed without taking care of it.  Let
> me come back to it in a sec, should be a minor change.

I've checked in the attached, to move making the cleanup out of the loop.
It doesn't need to be sooner, as the only thing we do before is checking
for args and throwing errors.



-- 
Pedro Alves

[-- Attachment #2: fix_thread_apply.diff --]
[-- Type: text/x-diff, Size: 889 bytes --]

2008-07-11  Pedro Alves  <pedro@codesourcery.com>

	* thread.c (thread_apply_command): Move making the cleanup out of
	the loop.

---
 gdb/thread.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c	2008-07-11 12:16:32.000000000 +0100
+++ src/gdb/thread.c	2008-07-11 12:23:43.000000000 +0100
@@ -1102,6 +1102,8 @@ thread_apply_command (char *tidlist, int
       else
 	end = start;
 
+      make_cleanup_restore_current_thread ();
+
       for (; start <= end; start++)
 	{
 	  tp = find_thread_id (start);
@@ -1112,8 +1114,6 @@ thread_apply_command (char *tidlist, int
 	    warning (_("Thread %d has terminated."), start);
 	  else
 	    {
-	      make_cleanup_restore_current_thread ();
-
 	      if (non_stop)
 		context_switch_to (tp->ptid);
 	      else