From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26866 invoked by alias); 14 Mar 2008 15:08:04 -0000 Received: (qmail 25334 invoked by uid 22791); 14 Mar 2008 15:07:54 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 14 Mar 2008 15:07:35 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id 0FF9B983B5; Fri, 14 Mar 2008 15:07:33 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id D8249983AA; Fri, 14 Mar 2008 15:07:32 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1JaBVM-0005OG-2T; Fri, 14 Mar 2008 11:07:32 -0400 Date: Fri, 14 Mar 2008 15:08:00 -0000 From: Daniel Jacobowitz To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] Implement -thread-info. Message-ID: <20080314150732.GA19288@caradoc.them.org> Mail-Followup-To: Vladimir Prus , gdb-patches@sources.redhat.com References: <200802171833.26673.vladimir@codesourcery.com> <200802181038.04497.vladimir@codesourcery.com> <18361.64577.771676.184618@kahikatea.snap.net.nz> <200802201427.36288.vladimir@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-12-11) X-IsSubscribed: yes 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-03/txt/msg00173.txt.bz2 On Sat, Mar 08, 2008 at 06:54:45PM +0300, Vladimir Prus wrote: > Vladimir Prus wrote: > > > I attach the revised version of the patch, including docs. Eli, is the > > doc part OK? I intend to commit the code part in a week unless there are > > objections. > > I have realized that while the patch is mostly about MI, it also > touches generic code -- thread.c. Is that part of patch (attached again > for convenience) OK? > > - Volodya > > * gdb.h (mi_info_threads): Declare. I think gdbthread.h is the appropriate header for this; gdb.h is for things using GDB_RC_OK and so forth. > @smallexample > - -thread-info > + -thread-info [ @var{thread-id} ] > @end smallexample > > +Reports the information about either a specific thread, if Just "reports information", no "the". > @@ -1055,6 +1073,7 @@ mi_cmd_list_features (char *command, char **argv, int argc) > > ui_out_field_string (uiout, NULL, "frozen-varobjs"); > ui_out_field_string (uiout, NULL, "pending-breakpoints"); > + ui_out_field_string (uiout, NULL, "thread-info"); > > do_cleanups (cleanup); > Shouldn't this be documented too? > +void > +mi_info_threads (struct ui_out *uiout, int requested_thread) Nick's right about the name of this function. print_thread_info would be fine. info_threads_1 would be fine too - the _1 is a weird convention that just means "the real body of info_threads", not "info_threads for one thread". print_thread_info sounds better to me. > /* Backup current thread and selected frame. */ > saved_frame_id = get_frame_id (get_selected_frame (NULL)); > old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id); > > + old_chain = make_cleanup_ui_out_list_begin_end (uiout, "threads"); > + By overwriting old_chain, you've stopped do_cleanups from restoring the previously selected thread. > extra_info = target_extra_thread_info (tp); > if (extra_info) > - printf_filtered (" (%s)", extra_info); > - puts_filtered (" "); > + ui_out_field_fmt (uiout, "details", " (%s)", extra_info); > + ui_out_text (uiout, " "); Do you really want to give the front end " (some extra text)"? It'll want to display this in a different column anyway. It should probably be ui_out_text (uiout, " ("); ui_out_field_string (uiout, extra_info); ui_out_text (") "); > /* That switch put us at the top of the stack (leaf frame). */ > switch_to_thread (tp->ptid); > print_stack_frame (get_selected_frame (NULL), 0, LOCATION); I think Nick was right about printing the frame level. It's not necessarily going to be zero in the future. For instance, GDB might automatically select the nearest user frame if you stop when a C++ exception is thrown, like we already do for Ada; and then we could teach GDB not to lose the selected frame whenever it switches threads. That wants doing anyway. -- Daniel Jacobowitz CodeSourcery