Re: [RFC] Implement -list-thread-groups.

Joel Brobecker
2008-11-14T01:53:02Z

> 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.

> void
> -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,
please?

> -
> +

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)
is -1?

--
Joel