This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patchv2] Sort threads for thread apply all (bt)


Jan Kratochvil writes:
 > On Thu, 15 Jan 2015 20:29:07 +0100, Doug Evans wrote:
 > > On Thu, Jan 15, 2015 at 10:33 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
 > > > I find maybe as good enough and with no risk of UI change flamewar to just
 > > > sort the threads by their number.  Currently they are printed as they happen
 > > > in the internal GDB list which has no advantage.  Printing thread #1 as the
 > > > first one with assumed 'thread apply all bt' (after the core file is loaded)
 > > > should make the complaint resolved I guess.
 > > >
 > > > No regressions on {x86_64,x86_64-m32,i686}-fedora22pre-linux-gnu.
 > > 
 > > No objection to sorting the list, but if thread #1 is the important one,
 > > then a concern could be it'll have scrolled off the screen (such a
 > > concern has been voiced in another thread in another context),
 > > and if not lost (say it's in an emacs buffer) one would still have
 > > to scroll back to see it.
 > > So one *could* still want #1 to be last.
 > > Do we want an option to choose the sort direction?
 > > [I wouldn't make it a global parameter, just an option to
 > > thread apply.]
 > 
 > Done.
 > 
 > 
 > Thanks,
 > Jan
 > gdb/ChangeLog
 > 2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
 > 
 > 	* thread.c (tp_array_compar_asc, tp_array_compar): New.
 > 	(thread_apply_all_command): Parse CMD for tp_array_compar_asc.  Sort
 > 	tp_array using tp_array_compar.
 > 	(_initialize_thread): Extend thread_apply_all_command help.
 > 
 > gdb/doc/ChangeLog
 > 2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
 > 
 > 	* gdb.texinfo (Threads): Describe -asc for thread apply all.

Hi.
Just some nits, no need to resubmit for review.

 > 
 > diff --git a/gdb/thread.c b/gdb/thread.c
 > index ed20fbe..9685351 100644
 > --- a/gdb/thread.c
 > +++ b/gdb/thread.c
 > @@ -1382,6 +1382,20 @@ make_cleanup_restore_current_thread (void)
 >  			    restore_current_thread_cleanup_dtor);
 >  }
 >  
 > +static int tp_array_compar_asc;

This should probably have a comment.

 > +
 > +/* Sort an array for struct thread_info pointers by their ascending NUM.  */
 > +
 > +static int
 > +tp_array_compar (const void *ap_voidp, const void *bp_voidp)
 > +{
 > +  const struct thread_info *const *ap = ap_voidp;
 > +  const struct thread_info *const *bp = bp_voidp;
 > +
 > +  return ((((*ap)->num > (*bp)->num) - ((*ap)->num < (*bp)->num))
 > +	  * (tp_array_compar_asc ? +1 : -1));
 > +}

This triggers my "passing parameters as global variables" alarm,
and while one could instead have two different functions,
this is ok, at least for now.

 > +
 >  /* Apply a GDB command to a list of threads.  List syntax is a whitespace
 >     seperated list of numbers, or ranges, or the keyword `all'.  Ranges consist
 >     of two numbers seperated by a hyphen.  Examples:
 > @@ -1398,6 +1412,13 @@ thread_apply_all_command (char *cmd, int from_tty)
 >    int tc;
 >    struct thread_array_cleanup ta_cleanup;
 >  
 > +  tp_array_compar_asc = 0;
 > +  if (cmd && (check_for_argument (&cmd, "-asc", strlen ("-asc"))))

cmd != NULL

 > +    {
 > +      cmd = skip_spaces (cmd);
 > +      tp_array_compar_asc = 1;
 > +    }
 > +
 >    if (cmd == NULL || *cmd == '\000')
 >      error (_("Please specify a command following the thread ID list"));
 >  
 > @@ -1431,6 +1452,8 @@ thread_apply_all_command (char *cmd, int from_tty)
 >            i++;
 >          }
 >  
 > +      qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
 > +
 >        make_cleanup (set_thread_refcount, &ta_cleanup);
 >  
 >        for (k = 0; k != i; k++)
 > @@ -1739,7 +1762,14 @@ The new thread ID must be currently known."),
 >  		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
 >  
 >    add_cmd ("all", class_run, thread_apply_all_command,
 > -	   _("Apply a command to all threads."), &thread_apply_list);
 > +	   _("\
 > +Apply a command to all threads.\n\
 > +\n\
 > +Usage: thread apply all [-asc] <command>\n\
 > +-asc: Call <command> for all threads in ascending order.\n\
 > +      The default is descending order.\n\

No final trailing newline.

 > +"),
 > +	   &thread_apply_list);
 >  
 >    add_cmd ("name", class_run, thread_name_command,
 >  	   _("Set the current thread's name.\n\
 > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
 > index f413e23..2207ce4 100644
 > --- a/gdb/doc/gdb.texinfo
 > +++ b/gdb/doc/gdb.texinfo
 > @@ -2959,14 +2959,17 @@ information on convenience variables.
 >  
 >  @kindex thread apply
 >  @cindex apply command to several threads
 > -@item thread apply [@var{threadno} | all] @var{command}
 > +@item thread apply [@var{threadno} | all [-asc]] @var{command}
 >  The @code{thread apply} command allows you to apply the named
 >  @var{command} to one or more threads.  Specify the numbers of the
 >  threads that you want affected with the command argument
 >  @var{threadno}.  It can be a single thread number, one of the numbers
 >  shown in the first field of the @samp{info threads} display; or it
 > -could be a range of thread numbers, as in @code{2-4}.  To apply a
 > -command to all threads, type @kbd{thread apply all @var{command}}.
 > +could be a range of thread numbers, as in @code{2-4}.  To apply
 > +a command to all threads in descending order, type @kbd{thread apply all
 > +@var{command}}.  To apply a command to all threads in ascending order,
 > +type @kbd{thread apply all -asc @var{command}}.
 > +
 >  
 >  @kindex thread name
 >  @cindex name a thread


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]