From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3215 invoked by alias); 16 Aug 2011 16:01:38 -0000 Received: (qmail 3203 invoked by uid 22791); 16 Aug 2011 16:01:36 -0000 X-SWARE-Spam-Status: No, hits=0.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_DYNAMIC,RCVD_IN_DNSWL_NONE,WEBMAIL_BODY X-Spam-Check-By: sourceware.org Received: from nm1-vm1.bt.bullet.mail.ukl.yahoo.com (HELO nm1-vm1.bt.bullet.mail.ukl.yahoo.com) (217.146.183.5) by sourceware.org (qpsmtpd/0.43rc1) with SMTP; Tue, 16 Aug 2011 16:01:21 +0000 Received: from [217.146.183.196] by nm1.bt.bullet.mail.ukl.yahoo.com with NNFMP; 16 Aug 2011 16:01:19 -0000 Received: from [217.146.183.204] by tm2.bt.bullet.mail.ukl.yahoo.com with NNFMP; 16 Aug 2011 16:01:19 -0000 Received: from [127.0.0.1] by omp1002.bt.mail.ukl.yahoo.com with NNFMP; 16 Aug 2011 16:01:19 -0000 Received: (qmail 5074 invoked by uid 60001); 16 Aug 2011 16:01:19 -0000 Received: from [94.72.254.2] by web86708.mail.ird.yahoo.com via HTTP; Tue, 16 Aug 2011 17:01:19 BST 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> <201108101610.31462.pedro@codesourcery.com> Message-ID: <1313510479.96560.YahooMailRC@web86708.mail.ird.yahoo.com> Date: Tue, 16 Aug 2011 16:01:00 -0000 From: pfee@talk21.com Subject: Re: Enhancement - show old and new thread info when switching during debugging To: Pedro Alves Cc: gdb-patches@sourceware.org, Joel Brobecker , Tom Tromey In-Reply-To: <201108101610.31462.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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/msg00334.txt.bz2 > > I tried kmail which allows setting the content-type and content-dispos= ition,=20 > > > but the email didn't reach the gdb-patches list. I'm reverting to us= ing=20 >webmail=20 > > > and I'll put the patch as plain text below. >=20 > Looks like the patch ends up line mangled that way. :-/ The email wou= ld > 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. In order to use kmail, I have to spoof my webmail address. I think somethi= ng is=20 detecting that and blocking the messages. Is this format ok, if not I'll s= witch=20 to an address that works with kmail. >=20 > > 2011-08-09 Paul Fee >=20 > Double space between your name and the email address too. >=20 > >=20 > > * inferior.h: Add $_prev_thread convenience variable. > > * infrun.c: Add $_prev_thread convenience variable, set it when cu= rrent=20 > > thread changes. > > * thread.c: Add $_prev_thread convenience variable, set it when us= er=20 > > switches threads. >=20 > 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: >=20 > * inferior.h (prev_selected_thread): Declare. > * infrun.c (prev_selected_thread): New global. > (prev_thread_id_make_value): New function. > ... etc. Still learning, thanks for the pointers. Unfortunately my webmail client e= ats=20 leading tabs, so I can't get the formatting perfect. >=20 > >=20 > > Index: inferior.h > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > 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 ---- > >=20=20 > > extern ptid_t inferior_ptid; > >=20=20 > > + /* 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; >=20 > Same comment formatting again. Typo s/interior/inferior/ >=20 > The comment is no longer correct. I'd go for not explaining the > implementation, but what the variable is supposed to track: Ok >=20 > /* The previously user/frontend selected thread. Exposed through the > $_prev_thread convenience variable. */ > extern ptid_t previous_selected_ptid; >=20 > and moving this declaration to gdbthread.h, and the definition > to thread.c. Ok >=20 > > Index: infrun.c > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > 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 =3D 0; > > *** 127,132 **** > > --- 127,138 ---- > >=20=20 > > static ptid_t previous_inferior_ptid; > >=20=20 > > + /* 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. */ >=20 > Comment duplication leads to bit rot. Please drop this copy. Ok >=20 > > +=20 > > + ptid_t previous_selected_ptid; > > +=20 > > /* Default behavior is to detach newly forked processes (legacy). */ > > int detach_fork =3D 1; > >=20=20 > > *************** print_no_history_reason (void) > > *** 5730,5735 **** > > --- 5736,5753 ---- > > ui_out_text (current_uiout, "\nNo more reverse-execution history.\= n"); > > } > >=20=20 > > + /* Return the previous thread's id. Return a value of 0 if > > + no previous thread was selected, or it doesn't exist. */ > > +=20 > > + struct value * > > + prev_thread_id_make_value (struct gdbarch *gdbarch, struct internalv= ar=20=20 >*var) > > + { > > + struct thread_info *tp =3D find_thread_ptid (previous_selected_pti= d); > > +=20 > > + return value_from_longest (builtin_type (gdbarch)->builtin_int, > > + (tp ? tp->num : 0)); > > + } >=20 > Please move this to thread.c, right next to thread_id_make_value, > and make it static. Ok >=20 > > +=20 > > /* Here to return control to GDB when the inferior stops for real. > > Print appropriate messages, remove breakpoints, give terminal our= =20=20 >modes. > >=20=20 > > *************** 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 =3D inferior_ptid; > > } > >=20=20 > > --- 5795,5803 ---- > > { > > target_terminal_ours_for_output (); > > printf_filtered (_("[Switching to %s]\n"), > > ! target_pid_to_str (inferior_ptid)); >=20 > I can't quite tell what changed in this line? I made some tab/space changes, I'll revert the change to make my patch smal= ler. >=20 > > annotate_thread_changed (); > > + previous_selected_ptid =3D previous_inferior_ptid; > > previous_inferior_ptid =3D inferior_ptid; >=20 > 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. The previous_selected_ptid remains populated when the program dies. When=20 the user accesses the convenience variable zero is returned as the=20 find_thread_ptid() call within prev_thread_id_make_value() returns NULL. H= ence we don't need to clear previous_selected_ptid. Do you agree? >=20 > > } >=20 > 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=20 > "... inferior" variants in inferior.c). I looked at this. I'm thinking that the previous_selected_ptid, rather tha= n=20 being a single global variable, should become a member of "struct inferior"= .=20=20 Therefore you could switch back and forth between inferiors and have=20 $_prev_thread updated to be specific to each inferior. Before make that chance I'd like your advice. Is that's what you had in mind and would this be an appropriate way to implement it? >=20 > Otherwise it's looking good. Thanks. Thanks for your help. Revised patch below. 2011-08-09 Paul Fee * gdbthread.h: Declare storage for new convenience variable. * infrun.c (normal_stop): Set $_prev_thread when stopping in different th= read. * thread.c: Define storage for new convenience variable. (do_captured_thread_select): Set $_prev_thread when user switches threads. (prev_thread_id_make_value): New function to lookup $_prev_thread. (_initialize_thread): Add $_prev_thread convenience variable. Index: gdbthread.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/src/src/gdb/gdbthread.h,v retrieving revision 1.64 diff -r1.64 gdbthread.h 228a229,232 > /* The previously user/frontend selected thread. Exposed through the > $_prev_thread convenience variable. */ > extern ptid_t previous_selected_ptid; >=20 Index: infrun.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.498 diff -r1.498 infrun.c 5781a5782 > previous_selected_ptid =3D previous_inferior_ptid; Index: thread.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/src/src/gdb/thread.c,v retrieving revision 1.142 diff -r1.142 thread.c 49a50,51 > ptid_t previous_selected_ptid; >=20 1396a1399 > previous_selected_ptid =3D inferior_ptid; 1451a1455,1467 > /* Return the previous thread's id. Return a value of 0 if > no previous thread was selected, or it doesn't exist. */ >=20 > static struct value * > prev_thread_id_make_value (struct gdbarch *gdbarch, struct internalvar *v= ar) > { > struct thread_info *tp =3D find_thread_ptid (previous_selected_ptid); >=20 > return value_from_longest (builtin_type (gdbarch)->builtin_int, > (tp ? tp->num : 0)); > } >=20 >=20 1500a1517 > create_internalvar_type_lazy ("_prev_thread", prev_thread_id_make_value= );