Re: [patch:MI] Observer for thread-changed

[Vladimir Prus <ghost@cs.msu.su>]

On Monday 09 June 2008 16:16:09 Nick Roberts wrote:
> 2008-06-09  Nick Roberts  <nickrob@snap.net.nz>
> 
>         * mi/mi-interp.c (mi_thread_changed): New static function.
>         (mi_interpreter_init): Register mi_thread_changed as thread_changed
>         observer .
>         * thread.c (thread_command): Notify thread changes using observer.
>         * infrun.c (normal_stop): Ditto.
>         * mi/mi-main.c (mi_cmd_thread_select): Ditto.
>         * Makefile.in (mi-main.o) : Add dependency on observer.h.
> 
> 2008-06-09  Nick Roberts  <nickrob@snap.net.nz>
> 
>         * observer.texi (GDB Observers): New observer for thread_changed.
> 
> 
> Index: thread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/thread.c,v
> retrieving revision 1.71
> diff -p -u -p -r1.71 thread.c
> --- thread.c    6 Jun 2008 00:32:51 -0000       1.71
> +++ thread.c    9 Jun 2008 12:13:23 -0000
> @@ -740,6 +740,7 @@ thread_command (char *tidstr, int from_t
>  
>    annotate_thread_changed ();
>    gdb_thread_select (uiout, tidstr, NULL);
> +  observer_notify_thread_changed ();
>  }
>  
>  /* Print notices when new threads are attached and detached.  */
> 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?

>  static void *
>  mi_interpreter_init (int top_level)
> @@ -92,6 +93,7 @@ mi_interpreter_init (int top_level)
>      {
>        observer_attach_new_thread (mi_new_thread);
>        observer_attach_thread_exit (mi_thread_exit);
> +      observer_attach_thread_changed (mi_thread_changed);
>      }
>  
>    return mi;
> @@ -331,6 +333,27 @@ mi_thread_exit (struct thread_info *t)
>    gdb_flush (mi->event_channel);
>  }
>  
> +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?

> +
>  extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */
>  
>  void
> Index: mi/mi-main.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
> retrieving revision 1.115
> diff -p -u -p -r1.115 mi-main.c
> --- mi/mi-main.c        6 May 2008 21:35:01 -0000       1.115
> +++ mi/mi-main.c        9 Jun 2008 12:13:28 -0000
> @@ -45,6 +45,7 @@
>  #include "frame.h"
>  #include "mi-main.h"
>  #include "language.h"
> +#include "observer.h"
>  
>  #include <ctype.h>
>  #include <sys/time.h>
> @@ -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.

OK with the above changes.

Are you planning to write tests, and document the new notification?

- Volodya