From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23304 invoked by alias); 10 Aug 2011 15:10:50 -0000 Received: (qmail 23293 invoked by uid 22791); 10 Aug 2011 15:10:49 -0000 X-SWARE-Spam-Status: No, hits=1.4 required=5.0 tests=AWL,BAYES_50,RP_MATCHES_RCVD,WEBMAIL_BODY X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 10 Aug 2011 15:10:34 +0000 Received: (qmail 27792 invoked from network); 10 Aug 2011 15:10:33 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 10 Aug 2011 15:10:33 -0000 From: Pedro Alves To: pfee@talk21.com Subject: Re: Enhancement - show old and new thread info when switching during debugging Date: Wed, 10 Aug 2011 15:10:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-10-generic; KDE/4.7.0; x86_64; ; ) Cc: gdb-patches@sourceware.org, Joel Brobecker , Tom Tromey References: <1311947955.89527.YahooMailRC@web86708.mail.ird.yahoo.com> <1312816830.93830.YahooMailRC@web86703.mail.ird.yahoo.com> <1312903533.54894.YahooMailRC@web86701.mail.ird.yahoo.com> In-Reply-To: <1312903533.54894.YahooMailRC@web86701.mail.ird.yahoo.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201108101610.31462.pedro@codesourcery.com> 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: 2011-08/txt/msg00222.txt.bz2 On Tuesday 09 August 2011 16:25:33, pfee@talk21.com wrote: > I tried kmail which allows setting the content-type and content-disposition, but > the email didn't reach the gdb-patches list. I'm reverting to using webmail and > I'll put the patch as plain text below. Looks like the patch ends up line mangled that way. :-/ The email would have been rejected if you sent as html instead of text. Otherwise, it's likely something with your smtp configuration in kmail. I use kmail every day with no such issue. > 2011-08-09 Paul Fee Double space between your name and the email address too. > > * inferior.h: Add $_prev_thread convenience variable. > * infrun.c: Add $_prev_thread convenience variable, set it when current > thread changes. > * thread.c: Add $_prev_thread convenience variable, set it when user > switches threads. Line mangling/wrapping here. Please keep lines under 80 columns. Align with a tab. Please see other changelog entries as guideline (or better the GNU coding standards chapter on Change Logs). Something like: * inferior.h (prev_selected_thread): Declare. * infrun.c (prev_selected_thread): New global. (prev_thread_id_make_value): New function. ... etc. > > Index: inferior.h > =================================================================== > RCS file: /cvs/src/src/gdb/inferior.h,v > retrieving revision 1.161 > diff -c -p -r1.161 inferior.h > *** inferior.h 21 Jul 2011 23:46:08 -0000 1.161 > --- inferior.h 9 Aug 2011 14:44:23 -0000 > *************** extern const char *get_inferior_io_termi > *** 94,99 **** > --- 94,105 ---- > > extern ptid_t inferior_ptid; > > + /* Values move from interior_ptid to previous_inferior_ptid to > + * previous_selected_ptid. The previous value is exposed to the > + * user through the $_prev_thread convenience variable. > + */ > + extern ptid_t previous_selected_ptid; Same comment formatting again. Typo s/interior/inferior/ The comment is no longer correct. I'd go for not explaining the implementation, but what the variable is supposed to track: /* The previously user/frontend selected thread. Exposed through the $_prev_thread convenience variable. */ extern ptid_t previous_selected_ptid; and moving this declaration to gdbthread.h, and the definition to thread.c. > Index: infrun.c > =================================================================== > RCS file: /cvs/src/src/gdb/infrun.c,v > retrieving revision 1.498 > diff -c -p -r1.498 infrun.c > *** infrun.c 4 Aug 2011 19:10:12 -0000 1.498 > --- infrun.c 9 Aug 2011 14:44:29 -0000 > *************** int sync_execution = 0; > *** 127,132 **** > --- 127,138 ---- > > static ptid_t previous_inferior_ptid; > > + /* Values move from interior_ptid to previous_inferior_ptid to > + previous_selected_ptid. The previous selected value is exposed > + to the user through the $_prev_thread convenience variable. */ Comment duplication leads to bit rot. Please drop this copy. > + > + ptid_t previous_selected_ptid; > + > /* Default behavior is to detach newly forked processes (legacy). */ > int detach_fork = 1; > > *************** print_no_history_reason (void) > *** 5730,5735 **** > --- 5736,5753 ---- > ui_out_text (current_uiout, "\nNo more reverse-execution history.\n"); > } > > + /* Return the previous thread's id. Return a value of 0 if > + no previous thread was selected, or it doesn't exist. */ > + > + struct value * > + prev_thread_id_make_value (struct gdbarch *gdbarch, struct internalvar *var) > + { > + struct thread_info *tp = find_thread_ptid (previous_selected_ptid); > + > + return value_from_longest (builtin_type (gdbarch)->builtin_int, > + (tp ? tp->num : 0)); > + } Please move this to thread.c, right next to thread_id_make_value, and make it static. > + > /* Here to return control to GDB when the inferior stops for real. > Print appropriate messages, remove breakpoints, give terminal our modes. > > *************** normal_stop (void) > *** 5777,5784 **** > { > target_terminal_ours_for_output (); > printf_filtered (_("[Switching to %s]\n"), > ! target_pid_to_str (inferior_ptid)); > annotate_thread_changed (); > previous_inferior_ptid = inferior_ptid; > } > > --- 5795,5803 ---- > { > target_terminal_ours_for_output (); > printf_filtered (_("[Switching to %s]\n"), > ! target_pid_to_str (inferior_ptid)); I can't quite tell what changed in this line? > annotate_thread_changed (); > + previous_selected_ptid = previous_inferior_ptid; > previous_inferior_ptid = inferior_ptid; I believe you should set or clear previous_selected_ptid on program exits too. This branch is only reached if the program is still alive. > } You'll need to handle a few more places. There's the "inferior" command -- see inferior.c and look for switch_to_thread calls; and I think "detach" and "kill" might need handling too(and the "... inferior" variants in inferior.c). Otherwise it's looking good. Thanks. -- Pedro Alves