[patchv3] Sort threads for thread apply all (bt)
Commit Message
On Thu, 22 Jan 2015 01:26:46 +0100, Doug Evans wrote:
> Just some nits, no need to resubmit for review.
Resubmitting it as a request for doc approval from Eli.
> > +static int tp_array_compar_asc;
>
> This should probably have a comment.
Done, I forgot.
> > +
> > +/* 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)
[...]
> > + 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,
I believe qsort_r() is more appropriate, using two different functions I would
feel rather as a workaround of missing qsort_r(). But I guess (I cannot easily
test due to missing slaves and no patch testing feature in Sergio's Buildbot
yet) some of the supported platforms do not provide qsort_r() so it would need
a new gdb/gnulib/ module. But at least one of my gdb/gnulib/ patches is still
under review so it would create dependency between unrelated patchsets.
> > + if (cmd && (check_for_argument (&cmd, "-asc", strlen ("-asc"))))
>
> cmd != NULL
BTW this has been copy-paste but yes, my fault, I am aware of it.
Thanks,
Jan
gdb/ChangeLog
2015-01-16 Jan Kratochvil <jan.kratochvil@redhat.com>
* thread.c (tp_array_compar_ascending, tp_array_compar): New.
(thread_apply_all_command): Parse CMD for tp_array_compar_ascending.
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 -ascending for thread apply all.
Comments
> Date: Thu, 22 Jan 2015 19:46:55 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches <gdb-patches@sourceware.org>, Doug Evans <dje@google.com>
>
> On Thu, 22 Jan 2015 01:26:46 +0100, Doug Evans wrote:
> > Just some nits, no need to resubmit for review.
>
> Resubmitting it as a request for doc approval from Eli.
Sorry for missing that. The documentation parts are OK.
(Do we want a NEWS entry?)
On Thu, Jan 22, 2015 at 10:46 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> ...
>> This triggers my "passing parameters as global variables" alarm,
>> and while one could instead have two different functions,
>
> I believe qsort_r() is more appropriate, using two different functions I would
> feel rather as a workaround of missing qsort_r(). But I guess (I cannot easily
> test due to missing slaves and no patch testing feature in Sergio's Buildbot
> yet) some of the supported platforms do not provide qsort_r() so it would need
> a new gdb/gnulib/ module. But at least one of my gdb/gnulib/ patches is still
> under review so it would create dependency between unrelated patchsets.
I think you're splitting hairs here.
> + if (cmd != NULL && (check_for_argument (&cmd, "-ascending", strlen ("-asc"))))
strlen ("-ascending")
Also, remove the extra parens around check_for_argument?
@@ -1381,6 +1381,24 @@ make_cleanup_restore_current_thread (void)
restore_current_thread_cleanup_dtor);
}
+/* If non-zero tp_array_compar should sort in ascending order, otherwise in
+ descending order. */
+
+static int tp_array_compar_ascending;
+
+/* Sort an array for struct thread_info pointers by their NUM, order is
+ determined by TP_ARRAY_COMPAR_ASCENDING. */
+
+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_ascending ? +1 : -1));
+}
+
/* 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:
@@ -1397,6 +1415,13 @@ thread_apply_all_command (char *cmd, int from_tty)
int tc;
struct thread_array_cleanup ta_cleanup;
+ tp_array_compar_ascending = 0;
+ if (cmd != NULL && (check_for_argument (&cmd, "-ascending", strlen ("-asc"))))
+ {
+ cmd = skip_spaces (cmd);
+ tp_array_compar_ascending = 1;
+ }
+
if (cmd == NULL || *cmd == '\000')
error (_("Please specify a command following the thread ID list"));
@@ -1430,6 +1455,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++)
@@ -1738,7 +1765,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 [-ascending] <command>\n\
+-ascending: Call <command> for all threads in ascending order.\n\
+ The default is descending order.\
+"),
+ &thread_apply_list);
add_cmd ("name", class_run, thread_name_command,
_("Set the current thread's name.\n\
@@ -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 [-ascending]] @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 -ascending @var{command}}.
+
@kindex thread name
@cindex name a thread