From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23537 invoked by alias); 9 Jun 2008 22:50:48 -0000 Received: (qmail 23526 invoked by uid 22791); 9 Jun 2008 22:50:47 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.25) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 09 Jun 2008 22:50:22 +0000 Received: from kahikatea.snap.net.nz (110.61.255.123.dynamic.snap.net.nz [123.255.61.110]) by viper.snap.net.nz (Postfix) with ESMTP id 880D73DA17E; Tue, 10 Jun 2008 10:50:19 +1200 (NZST) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id 14ACB8FC6D; Tue, 10 Jun 2008 10:50:06 +1200 (NZST) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Message-ID: <18509.45981.233865.890248@kahikatea.snap.net.nz> Date: Mon, 09 Jun 2008 23:35:00 -0000 To: Pedro Alves Cc: gdb-patches@sourceware.org, ghost@cs.msu.su Subject: Re: [patch:MI] Observer for thread-changed In-Reply-To: <200806091428.29157.pedro@codesourcery.com> References: <18509.7945.19078.399646@kahikatea.snap.net.nz> <200806091428.29157.pedro@codesourcery.com> X-Mailer: VM 7.19 under Emacs 22.2.50.2 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-06/txt/msg00175.txt.bz2 Pedro Alves writes: > Sorry, if I missed the discussion on it, but, >=20 > A Monday 09 June 2008 13:16:09, Nick Roberts wrote: > > =A0 =A0annotate_thread_changed (); > > =A0 =A0gdb_thread_select (uiout, tidstr, NULL); > > + =A0observer_notify_thread_changed (); > > =A0} >=20 > This is conceptually not right. gdb_thread_select is a libgdb > function, that filters exceptions. If do_captured_thread_select > throws an error, you will still call the observer. Plus,=20 > do_captured_thread_select is already printing the thread change > to MI, which means you'll get the output twice now, in MI? I don't think that's a problem. Removing the output from -thread-select would make it backwardly incompatible. > Why not call the observer from inside do_captured_thread_select, > instead of on both CLI and MI commands? Yes, you're right. Presumably I could also put it in a clause in gdb_thread_select? I did have it in do_captured_thread_select in the first patch but I moved it. I can't fully explain why now but I think I must have got confused by output of the frame_changed observer, which was also part of that patch, being triggered by "info threads". I'll have to move the call to annotate_thread_changed too. > > /* Print notices when new threads are attached and detached. */ > > 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.278 > > diff -p -u -p -r1.278 infrun.c > > --- infrun.c 6 Jun 2008 00:33:52 -0000 1.278 > > +++ infrun.c 9 Jun 2008 12:13:25 -0000 > > @@ -3605,6 +3605,7 @@ normal_stop (void) > > target_terminal_ours_for_output (); > > printf_filtered (_("[Switching to %s]\n"), > > target_pid_to_str (inferior_ptid)); > > + observer_notify_thread_changed (); > > annotate_thread_changed (); > > previous_inferior_ptid =3D inferior_ptid; > > } > Hmm, will we want the observer to have access to the selected frame > of the new selected thread? If so, then, the observer call should be > moved to the end of normal_stop after the "done:" label, > so any dummy frame is poped; if not, then I guess a comment here would > be good, as it seems something natural to be doing from inside > the observer in the future. Doesn't GDB already output the selected frame with "*stopped" > Also, it may make sense to add a "reason" parameter to > the observer, as in "changed due to user/frontend request", or > "due to a stop event", but that's not actually required right now. I'm not sure what use this information would be. If it's due to a stop eve= nt then the reason should be given in the async output. How about the change below instead? This, of course, requires no change to mi-main.c. --=20 Nick http://www.inet.net.nz/~nick= rob --- thread.c 09 Jun 2008 21:06:46 +1200 1.71 +++ thread.c 10 Jun 2008 01:37:12 +1200=09 @@ -738,8 +738,8 @@ thread_command (char *tidstr, int from_t return; } =20 - annotate_thread_changed (); gdb_thread_select (uiout, tidstr, NULL); + observer_notify_thread_changed (); } =20 /* Print notices when new threads are attached and detached. */ @@ -786,8 +786,13 @@ gdb_thread_select (struct ui_out *uiout, { if (catch_exceptions_with_msg (uiout, do_captured_thread_select, tidstr, error_message, RETURN_MASK_ALL) < 0) - return GDB_RC_FAIL; - return GDB_RC_OK; + return GDB_RC_FAIL; + else + { + observer_notify_thread_changed (); + annotate_thread_changed (); + return GDB_RC_OK; + } } =20 /* Commands with a prefix of `thread'. */