This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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

> -
> +	

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?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]