This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch:MI] Observer for thread-changed
> > Index: infrun.c
> > ===================================================================
> > 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 = inferior_ptid;
> > ? ? ?}
>
> 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 ()
> > +{
> > + ?struct mi_interp *mi = top_level_interpreter_data ();
> > + ?struct interp *interp_to_use;
> > + ?struct ui_out *old_uiout, *temp_uiout;
> > + ?int version;
> > +
> > + ?fprintf_unfiltered (mi->event_channel, "thread-changed");
> > + ?interp_to_use = top_level_interpreter ();
> > + ?old_uiout = uiout;
> > + ?temp_uiout = interp_ui_out (interp_to_use);
> > + ?version = mi_version (temp_uiout);
> > + ?temp_uiout = mi_out_new (version);
> > + ?uiout = temp_uiout;
> > + ?ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
> > + ?mi_out_put (uiout, mi->event_channel);
> > + ?uiout = old_uiout;
> > + ?gdb_flush (mi->event_channel);
> > +}
>
> Since your other patches have similar code, how about introducing a helper
> 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
> > ? ? ?error ("mi_cmd_thread_select: USAGE: threadnum.");
> > ?
> > ? ?rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
> > + ?observer_notify_thread_changed ();
>
> 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 would
mean that -thread-select and the CLI "thread" command are processed the same
way.
> OK with the above changes.
>
> 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".
--
Nick http://www.inet.net.nz/~nickrob