On Friday 14 November 2008 04:52:17 Joel Brobecker wrote:
> > 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.

Sorry if I'm trying to circumvent the rules. On the other hand, I believe I 
was told that MI boundaries are logical, not physical. In that sense, a change 
that clearly matters only for MI, and is actually posted for comments, is kind
of a boundary case.

Another example are observes. Although generic in nature, they are quickly
becoming de-facto interface of GDB core with MI.

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

Done.

>
> > -
> > +
>
> You're adding some spaces in what would otherwise be an empty line.
> Can you get rid of that change?

Done. Indicentally, I know how to configure Emacs to remove trailing 
whitespace on save. But pretty much every source file in gdb already has such 
lines. If somebody tell me how to make Emacs not add lines without trailing
whitespace, while *not* changing existing lines, it would be gtreat.

> > +  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?

Yes, exactly. If you ask to print details of a specific thread, then you don't
care what current thread is.

I attach the revised patch.

- Volodya