From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23060 invoked by alias); 10 Jun 2008 08:26:01 -0000 Received: (qmail 23040 invoked by uid 22791); 10 Jun 2008 08:26:00 -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; Tue, 10 Jun 2008 08:25:32 +0000 Received: from kahikatea.snap.net.nz (215.60.255.123.dynamic.snap.net.nz [123.255.60.215]) by viper.snap.net.nz (Postfix) with ESMTP id AC4FB3DA162; Tue, 10 Jun 2008 20:25:28 +1200 (NZST) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id B82608FC6D; Tue, 10 Jun 2008 20:25:12 +1200 (NZST) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Message-ID: <18510.14942.143700.410083@kahikatea.snap.net.nz> Date: Tue, 10 Jun 2008 09:19:00 -0000 To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Subject: Re: [patch:MI] Observer for thread-changed In-Reply-To: <200806101057.19272.ghost@cs.msu.su> References: <18509.7945.19078.399646@kahikatea.snap.net.nz> <200806101057.19272.ghost@cs.msu.su> 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/msg00193.txt.bz2 > > 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=A0=A0=A0=A06 Jun 2008 00:33:52 -0000=A0=A0=A0=A0=A0=A0=A0= 1.278 > > +++ infrun.c=A0=A0=A0=A09 Jun 2008 12:13:25 -0000 > > @@ -3605,6 +3605,7 @@ normal_stop (void) > > =A0 =A0 =A0 =A0target_terminal_ours_for_output (); > > =A0 =A0 =A0 =A0printf_filtered (_("[Switching to %s]\n"), > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0 =A0 target_pi= d_to_str (inferior_ptid)); > > + =A0 =A0 =A0observer_notify_thread_changed (); > > =A0 =A0 =A0 =A0annotate_thread_changed (); > > =A0 =A0 =A0 =A0previous_inferior_ptid =3D inferior_ptid; > > =A0 =A0 =A0} >=20 > Pedro's has asked if we'd want to call the observer only when we've > selected a frame in the new thread. Have you decided if that's a good > idea or not? My observer doesn't use the selected frame, so I don't have an opinion. > > ... > > +static void > > +mi_thread_changed () > > +{ > > + =A0struct mi_interp *mi =3D top_level_interpreter_data (); > > + =A0struct interp *interp_to_use; > > + =A0struct ui_out *old_uiout, *temp_uiout; > > + =A0int version; > > + > > + =A0fprintf_unfiltered (mi->event_channel, "thread-changed"); > > + =A0interp_to_use =3D top_level_interpreter (); > > + =A0old_uiout =3D uiout; > > + =A0temp_uiout =3D interp_ui_out (interp_to_use); > > + =A0version =3D mi_version (temp_uiout); > > + =A0temp_uiout =3D mi_out_new (version); > > + =A0uiout =3D temp_uiout; > > + =A0ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (infer= ior_ptid)); > > + =A0mi_out_put (uiout, mi->event_channel); > > + =A0uiout =3D old_uiout; > > + =A0gdb_flush (mi->event_channel); > > +} >=20 > Since your other patches have similar code, how about introducing a help= er > function for creating temporary uiout, as part of this patch? Refactoring requires pointers to functions and function arguments and makes= the code harder to read. I am happy to do this when there is more than one such function. > > @@ -241,6 +242,7 @@ mi_cmd_thread_select (char *command, cha > > =A0 =A0 =A0error ("mi_cmd_thread_select: USAGE: threadnum."); > > =A0 > > =A0 =A0rc =3D gdb_thread_select (uiout, argv[0], &mi_error_message); > > + =A0observer_notify_thread_changed (); >=20 > As I've explained in the other email, I think that thread-changed > notification should not be emitted for -thread-select, since the frontend > does not need notification about something it just explicitly did > itself. So, this call probably should disappear. When Emacs migrates fully to MI I anticipate it would just process the the notification and ignore the direct output from -thread-select. That wo= uld mean that -thread-select and the CLI "thread" command are processed the same way. > OK with the above changes. >=20 > Are you planning to write tests, and document the new notification? There certainly should be tests and documentation, and I will do write some, but I don't see any for the existing notifications, namely "thread-created"= and "thread-exited". --=20 Nick http://www.inet.net.nz/~nick= rob