From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31862 invoked by alias); 14 Nov 2008 17:29:40 -0000 Received: (qmail 31659 invoked by uid 22791); 14 Nov 2008 17:29:39 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 14 Nov 2008 17:28:53 +0000 Received: (qmail 17765 invoked from network); 14 Nov 2008 17:28:50 -0000 Received: from unknown (HELO wind.localnet) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 14 Nov 2008 17:28:50 -0000 From: Vladimir Prus To: Joel Brobecker Subject: Re: [RFC] Implement -list-thread-groups. Date: Fri, 14 Nov 2008 19:43:00 -0000 User-Agent: KMail/1.10.90 (Linux/2.6.24-21-generic; KDE/4.1.73; i686; svn-883826; 2008-11-13) Cc: gdb-patches@sources.redhat.com References: <200811122333.29218.vladimir@codesourcery.com> <20081114015217.GD12802@adacore.com> In-Reply-To: <20081114015217.GD12802@adacore.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_LVbHJ9dMBHY08xf" Message-Id: <200811142028.43561.vladimir@codesourcery.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-11/txt/msg00357.txt.bz2 --Boundary-00=_LVbHJ9dMBHY08xf Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 2553 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 --Boundary-00=_LVbHJ9dMBHY08xf Content-Type: text/x-patch; charset="UTF-8"; name="list_thread_groups.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="list_thread_groups.patch" Content-length: 5515 commit a28a9fa5263190c36b5899f17b5746fd8cc292a2 Author: vladimir Date: Thu Jul 31 13:23:28 2008 +0000 Implement -list-thread-groups. * 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. diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index 55c848d..cac20f7 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -295,7 +295,8 @@ extern struct cmd_list_element *thread_cmd_list; `set print thread-events'. */ extern int print_thread_events; -extern void print_thread_info (struct ui_out *uiout, int thread); +extern void print_thread_info (struct ui_out *uiout, int thread, + int pid); extern struct cleanup *make_cleanup_restore_current_thread (void); diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c index ca0f428..d38de35 100644 --- a/gdb/mi/mi-cmds.c +++ b/gdb/mi/mi-cmds.c @@ -90,6 +90,7 @@ struct mi_cmd mi_cmds[] = { "interpreter-exec", { NULL, 0 }, mi_cmd_interpreter_exec}, { "list-features", { NULL, 0 }, mi_cmd_list_features}, { "list-target-features", { NULL, 0 }, mi_cmd_list_target_features}, + { "list-thread-groups", { NULL, 0 }, mi_cmd_list_thread_groups }, { "overlay-auto", { NULL, 0 }, NULL }, { "overlay-list-mapping-state", { NULL, 0 }, NULL }, { "overlay-list-overlays", { NULL, 0 }, NULL }, diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h index 16887ae..a9bb1e0 100644 --- a/gdb/mi/mi-cmds.h +++ b/gdb/mi/mi-cmds.h @@ -67,6 +67,7 @@ extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show; extern mi_cmd_argv_ftype mi_cmd_interpreter_exec; extern mi_cmd_argv_ftype mi_cmd_list_features; extern mi_cmd_argv_ftype mi_cmd_list_target_features; +extern mi_cmd_argv_ftype mi_cmd_list_thread_groups; extern mi_cmd_argv_ftype mi_cmd_stack_info_depth; extern mi_cmd_argv_ftype mi_cmd_stack_info_frame; extern mi_cmd_argv_ftype mi_cmd_stack_list_args; diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index a9fbcad..544fec6 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -46,6 +46,7 @@ #include "mi-main.h" #include "language.h" #include "valprint.h" +#include "processes.h" #include #include @@ -245,7 +246,55 @@ mi_cmd_thread_info (char *command, char **argv, int argc) if (argc == 1) thread = atoi (argv[0]); - print_thread_info (uiout, thread); + print_thread_info (uiout, thread, -1); +} + +static int +print_one_process (struct process_info *process, void *arg) +{ + struct cleanup *back_to = make_cleanup_ui_out_tuple_begin_end (uiout, NULL); + + ui_out_field_fmt (uiout, "id", "p%d", process->pid); + ui_out_field_string (uiout, "type", "process"); + ui_out_field_int (uiout, "pid", process->pid); + + do_cleanups (back_to); + return 0; +} + +void +mi_cmd_list_thread_groups (char *command, char **argv, int argc) +{ + struct cleanup *back_to; + int available = 0; + char *id = NULL; + + if (argc > 0 && strcmp (argv[0], "--available") == 0) + { + ++argv; + --argc; + available = 1; + } + + if (argc > 0) + id = argv[0]; + + back_to = make_cleanup (&null_cleanup, NULL); + + if (id) + { + int pid = atoi (id); + if (!in_process_list (pid)) + error ("Invalid thread group id '%s'", id); + print_thread_info (uiout, -1, pid); + } + else + { + make_cleanup_ui_out_list_begin_end (uiout, "groups"); + iterate_over_processes (print_one_process, NULL); + } + + do_cleanups (back_to); } void diff --git a/gdb/thread.c b/gdb/thread.c index b1e318d..a32dff4 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -636,9 +636,10 @@ set_stop_requested (ptid_t ptid, int stop) use from MI. If REQUESTED_THREAD is not -1, it's the GDB id of the thread that should be printed. Otherwise, all threads are - printed. */ + printed. + If PID is not -1, only prints threads from the process PID. */ void -print_thread_info (struct ui_out *uiout, int requested_thread) +print_thread_info (struct ui_out *uiout, int requested_thread, int pid) { struct thread_info *tp; ptid_t current_ptid; @@ -646,6 +647,8 @@ print_thread_info (struct ui_out *uiout, int requested_thread) char *extra_info; int current_thread = -1; + gdb_assert (requested_thead == -1 || pid == -1); + prune_threads (); target_find_new_threads (); current_ptid = inferior_ptid; @@ -658,6 +661,9 @@ print_thread_info (struct ui_out *uiout, int requested_thread) { struct cleanup *chain2; + if (pid != -1 && PIDGET (tp->ptid) != pid) + continue; + if (requested_thread != -1 && tp->num != requested_thread) continue; @@ -715,7 +721,7 @@ print_thread_info (struct ui_out *uiout, int requested_thread) the "info threads" command. */ do_cleanups (old_chain); - if (requested_thread == -1) + if (pid == -1 && requested_thread == -1 ) { gdb_assert (current_thread != -1 || !thread_list); @@ -740,7 +746,7 @@ The current thread has terminated. See `help thread'.\n", static void info_threads_command (char *arg, int from_tty) { - print_thread_info (uiout, -1); + print_thread_info (uiout, -1, -1); } /* Switch from one thread to another. */ --Boundary-00=_LVbHJ9dMBHY08xf--