Re: [patch] info threads sort by name and name regex matching

[Aaron Gamble <agamble at google dot com>]

On Wed, Aug 22, 2012 at 11:51 AM, Tom Tromey <tromey@redhat.com> wrote:

*snip*

> Aaron> With this patch sorting by name will happen always. (Perhaps a switch
> Aaron> to enable sorting would be better?)
>
> Yes, I think so.  I think sorting by name makes sense but it isn't
> perhaps always what you want.  Sorting by number has the nice feature
> that it is stable.
>
> Aaron> Regex matching is specified by doing 'info threads r<regex>'. This is
> Aaron> not ambiguous with previous behavior where parameters to info threads
> Aaron> were only numbers. (spaces between 'r' and <regex> are ignored)
>
> I'm curious why you chose this particular spelling.
> Other possible approaches would be a subcommand, or a flag like "-r".
> Either of these is perhaps more in keeping with gdb tradition.
>
> I'm interested in other opinions here too.
>
> Expect some bikeshedding on this point.
I picked a single letter for it to be quick and easy to type. I
suppose it's possible 'info threads' could be expanded to do other
sorts of matching where different flags would be useful.

How about 'info threads [ -a ] [ id.. | -n <name regex> ]'

-a - Sort alphabetically by name
-n regex - Match thread names with regex

Of course then we could do sorting based on the name of the function a
thread is in or other sorts of sorting. So other sorting flags would
need to be introduced.

*snip*
> Aaron> +      threads = xmalloc (sizeof (*threads) * thread_list_size);
> Aaron> +      make_cleanup (free, threads);
>
> I think making a VEC here would be better.
> Then you wouldn't need thread_list_size at all.

Hmm, not sure if this would improve performance at all. A one time
allocation bounded by the number of threads vs VEC's implementation of
expanding arrays. I'll wait for others feedback.

>
> Tom

Thanks for the feedback. I'll fix the other things you mentioned I snipped out.


-Aaron