This is the mail archive of the
mailing list for the GDB project.
Re: [RFC] Implement -list-thread-groups.
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Vladimir Prus <vladimir at codesourcery dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Thu, 13 Nov 2008 17:52:17 -0800
- Subject: Re: [RFC] Implement -list-thread-groups.
- References: <firstname.lastname@example.org>
> I'll commit in a few days if there are no objections.
If others are fine with this too, then so am I. But I don't think
this is the normal procedure: For non-MI changes, my understanding
is that you're supposed to wait for approval unless the changes
are obvious (particularly for patches labeled RFC). I don't blame
you for doing this, given the poor track record we're having in
terms of speed of review - I guess we need more help.
Anyway, onto the patch...
> * thread.c (print_thread_info): New parameter pid, to print
> threads of specific process.
> * gdbthread.h (print_thread_info): New parameter pid.
> * mi/mi-cmds.c (mi_cmds): Register -list-thread-groups.
> * mi/mi-cmds.h (mi_cmd_list_thread_groups): New.
> * mi/mi-main.c (mi_cmd_thread_info): Adjust.
> (print_one_process, mi_cmd_list_thread_groups): New.
Overall, looks good to me. Just a couple of minor nits and a question.
> -print_thread_info (struct ui_out *uiout, int requested_thread)
> +print_thread_info (struct ui_out *uiout, int requested_thread, int pid)
Can you add a comment for the new parameter in the function documentation,
You're adding some spaces in what would otherwise be an empty line.
Can you get rid of that change?
> + if (pid == -1 && requested_thread == -1 )
> gdb_assert (current_thread != -1
> || !thread_list);
This has little to do with your change, but I'm curious. What does
this code do? It seems to be adding a current-thread-id field in
the output, but why only doing it when requested_thread (and pid)