RFC: change MI event channel to use ui-out

[Tom Tromey <tromey at redhat dot com>]

This patch changes MI's event channel from a ui-file to a ui-out.

The rationale for this comes in a later patch: I wanted to emit some
async breakpoint notifications, and it seemed nicer to reuse the
existing breakpoint-printing functions, which all use ui-out objects.

However, I think this is a reasonable cleanup on its own.  Its primary
plus is that it makes it more difficult to emit incorrect output.  E.g.,
right now, if a library name contains a double quote, I think gdb will
print the wrong this, but after this patch, the output will be correct.q

This change required a little hackery in the MI ui-out object itself.
I changed it to format top-level lists slightly differently in the event
channel case.

Built and regtested on x86-64 (compile farm).

Let me know what you think.  In the absence of comments I will commit
this after a decent interval.

Tom

2011-01-26  Tom Tromey  <tromey@redhat.com>

	* mi/mi-out.h (mi_out_new): Update.
	* mi/mi-out.c (struct ui_out_data) <for_events, list_depth>: New
	fields.
	(mi_open): Handle new fields.
	(mi_close): Likewise.
	(mi_out_new): Add 'out' argument.  Update for new fields.
	* mi/mi-main.c (mi_execute_command): Update.
	(mi_load_progress): Update.
	* mi/mi-interp.c (mi_interpreter_init): Update.
	(mi_new_thread): Update.
	(mi_thread_exit): Likewise.
	(mi_inferior_added): Likewise.
	(mi_inferior_appeared): Likewise.
	(mi_inferior_exit): Likewise.
	(mi_inferior_removed): Likewise.
	(mi_solib_loaded): Likewise.
	(mi_solib_unloaded): Likewise.
	(report_initial_inferior): Likewise.
	(_initialize_mi_interp): Likewise.
	* mi/mi-common.h (struct mi_interp) <event_channel>: New a
	ui_out*.

diff --git a/gdb/mi/mi-common.h b/gdb/mi/mi-common.h
index e3aab7d..6df1678 100644
--- a/gdb/mi/mi-common.h
+++ b/gdb/mi/mi-common.h
@@ -50,7 +50,7 @@ struct mi_interp
   struct ui_file *err;
   struct ui_file *log;
   struct ui_file *targ;
-  struct ui_file *event_channel;
+  struct ui_out *event_channel;
 
   /* This is the interpreter for the mi... */
   struct interp *mi2_interp;
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 23c60f6..ea69c75 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -84,7 +84,8 @@ mi_interpreter_init (int top_level)
   mi->err = mi_console_file_new (raw_stdout, "&", '"');
   mi->log = mi->err;
   mi->targ = mi_console_file_new (raw_stdout, "@", '"');
-  mi->event_channel = mi_console_file_new (raw_stdout, "=", 0);
+  mi->event_channel = mi_out_new (3 /* This doesn't actually matter.  */,
+				  mi_console_file_new (raw_stdout, "=", 0));
 
   if (top_level)
     {
@@ -301,13 +302,15 @@ mi_new_thread (struct thread_info *t)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
   struct inferior *inf = find_inferior_pid (ptid_get_pid (t->ptid));
+  struct cleanup *cleanup;
 
   gdb_assert (inf);
 
-  fprintf_unfiltered (mi->event_channel, 
-		      "thread-created,id=\"%d\",group-id=\"i%d\"",
-		      t->num, inf->num);
-  gdb_flush (mi->event_channel);
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"thread_created");
+  ui_out_field_int (mi->event_channel, "id", t->num);
+  ui_out_field_fmt (mi->event_channel, "group-id", "i%d", inf->num);
+  do_cleanups (cleanup);
 }
 
 static void
@@ -315,6 +318,7 @@ mi_thread_exit (struct thread_info *t, int silent)
 {
   struct mi_interp *mi;
   struct inferior *inf;
+  struct cleanup *cleanup;
 
   if (silent)
     return;
@@ -323,57 +327,64 @@ mi_thread_exit (struct thread_info *t, int silent)
 
   mi = top_level_interpreter_data ();
   target_terminal_ours ();
-  fprintf_unfiltered (mi->event_channel, 
-		      "thread-exited,id=\"%d\",group-id=\"i%d\"",
-		      t->num, inf->num);
-  gdb_flush (mi->event_channel);
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"thread-exited");
+  ui_out_field_int (mi->event_channel, "id", t->num);
+  ui_out_field_fmt (mi->event_channel, "group-id", "i%d", inf->num);
+  do_cleanups (cleanup);
 }
 
 static void
 mi_inferior_added (struct inferior *inf)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
+  struct cleanup *cleanup;
 
   target_terminal_ours ();
-  fprintf_unfiltered (mi->event_channel,
-		      "thread-group-added,id=\"i%d\"",
-		      inf->num);
-  gdb_flush (mi->event_channel);
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"thread-group-added");
+  ui_out_field_fmt (mi->event_channel, "id", "i%d", inf->num);
+  do_cleanups (cleanup);
 }
 
 static void
 mi_inferior_appeared (struct inferior *inf)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
+  struct cleanup *cleanup;
 
   target_terminal_ours ();
-  fprintf_unfiltered (mi->event_channel,
-		      "thread-group-started,id=\"i%d\",pid=\"%d\"",
-		      inf->num, inf->pid);
-  gdb_flush (mi->event_channel);
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"thread-group-started");
+  ui_out_field_fmt (mi->event_channel, "id", "i%d", inf->num);
+  ui_out_field_fmt (mi->event_channel, "pid", "%d", inf->pid);
+  do_cleanups (cleanup);
 }
 
 static void
 mi_inferior_exit (struct inferior *inf)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
+  struct cleanup *cleanup;
 
   target_terminal_ours ();
-  fprintf_unfiltered (mi->event_channel, "thread-group-exited,id=\"i%d\"",
-		      inf->num);
-  gdb_flush (mi->event_channel);  
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"thread-group-exited");
+  ui_out_field_fmt (mi->event_channel, "id", "i%d", inf->num);
+  do_cleanups (cleanup);
 }
 
 static void
 mi_inferior_removed (struct inferior *inf)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
+  struct cleanup *cleanup;
 
   target_terminal_ours ();
-  fprintf_unfiltered (mi->event_channel,
-		      "thread-group-removed,id=\"i%d\"",
-		      inf->num);
-  gdb_flush (mi->event_channel);
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"thread-group-removed");
+  ui_out_field_fmt (mi->event_channel, "id", "i%d", inf->num);
+  do_cleanups (cleanup);
 }
 
 static void
@@ -540,62 +551,70 @@ static void
 mi_solib_loaded (struct so_list *solib)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
+  struct cleanup *cleanup;
 
   target_terminal_ours ();
-  if (gdbarch_has_global_solist (target_gdbarch))
-    fprintf_unfiltered (mi->event_channel,
-			"library-loaded,id=\"%s\",target-name=\"%s\","
-			"host-name=\"%s\",symbols-loaded=\"%d\"",
-			solib->so_original_name, solib->so_original_name,
-			solib->so_name, solib->symbols_loaded);
-  else
-    fprintf_unfiltered (mi->event_channel,
-			"library-loaded,id=\"%s\",target-name=\"%s\","
-			"host-name=\"%s\",symbols-loaded=\"%d\","
-			"thread-group=\"i%d\"",
-			solib->so_original_name, solib->so_original_name,
-			solib->so_name, solib->symbols_loaded,
-			current_inferior ()->num);
-
-  gdb_flush (mi->event_channel);
+
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"library-loaded");
+
+  ui_out_field_string (mi->event_channel, "id",
+		       solib->so_original_name);
+  ui_out_field_string (mi->event_channel, "target-name",
+		       solib->so_original_name);
+  ui_out_field_string (mi->event_channel, "host-name",
+		       solib->so_name);
+  ui_out_field_int (mi->event_channel, "symbols-loaded",
+		    solib->symbols_loaded);
+
+  if (! gdbarch_has_global_solist (target_gdbarch))
+    ui_out_field_fmt (mi->event_channel, "thread-group", "i%d",
+		      current_inferior ()->num);
+
+  do_cleanups (cleanup);
 }
 
 static void
 mi_solib_unloaded (struct so_list *solib)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
+  struct cleanup *cleanup;
 
   target_terminal_ours ();
-  if (gdbarch_has_global_solist (target_gdbarch))
-    fprintf_unfiltered (mi->event_channel,
-			"library-unloaded,id=\"%s\",target-name=\"%s\","
-			"host-name=\"%s\"",
-			solib->so_original_name, solib->so_original_name,
-			solib->so_name);
-  else
-    fprintf_unfiltered (mi->event_channel,
-			"library-unloaded,id=\"%s\",target-name=\"%s\","
-			"host-name=\"%s\",thread-group=\"i%d\"",
-			solib->so_original_name, solib->so_original_name,
-			solib->so_name, current_inferior ()->num);
 
-  gdb_flush (mi->event_channel);
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"library-unloaded");
+  ui_out_field_string (mi->event_channel, "id",
+		       solib->so_original_name);
+  ui_out_field_string (mi->event_channel, "target-name",
+		       solib->so_original_name);
+  ui_out_field_string (mi->event_channel, "host-name",
+		       solib->so_name);
+
+  if (! gdbarch_has_global_solist (target_gdbarch))
+    ui_out_field_fmt (mi->event_channel, "thread-group", "i%d",
+		      current_inferior ()->num);
+
+  do_cleanups (cleanup);
 }
 
 static int
 report_initial_inferior (struct inferior *inf, void *closure)
 {
-  /* This function is called from mi_intepreter_init, and since
+  /* This function is called from mi_interpreter_init, and since
      mi_inferior_added assumes that inferior is fully initialized
      and top_level_interpreter_data is set, we cannot call
      it here.  */
   struct mi_interp *mi = closure;
+  struct cleanup *cleanup;
 
   target_terminal_ours ();
-  fprintf_unfiltered (mi->event_channel,
-		      "thread-group-added,id=\"i%d\"",
-		      inf->num);
-  gdb_flush (mi->event_channel);
+
+  cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						"thread-group-added");
+  ui_out_field_fmt (mi->event_channel, "added", "i%d", inf->num);
+  do_cleanups (cleanup);
+
   return 0;
 }
 
@@ -614,11 +633,11 @@ _initialize_mi_interp (void)
   };
 
   /* The various interpreter levels.  */
-  interp_add (interp_new (INTERP_MI1, NULL, mi_out_new (1), &procs));
-  interp_add (interp_new (INTERP_MI2, NULL, mi_out_new (2), &procs));
-  interp_add (interp_new (INTERP_MI3, NULL, mi_out_new (3), &procs));
+  interp_add (interp_new (INTERP_MI1, NULL, mi_out_new (1, NULL), &procs));
+  interp_add (interp_new (INTERP_MI2, NULL, mi_out_new (2, NULL), &procs));
+  interp_add (interp_new (INTERP_MI3, NULL, mi_out_new (3, NULL), &procs));
 
   /* "mi" selects the most recent released version.  "mi2" was
      released as part of GDB 6.0.  */
-  interp_add (interp_new (INTERP_MI, NULL, mi_out_new (2), &procs));
+  interp_add (interp_new (INTERP_MI, NULL, mi_out_new (2, NULL), &procs));
 }
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 1f1b712..d9e988c 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2004,12 +2004,15 @@ mi_execute_command (char *cmd, int from_tty)
 	  if (report_change)
 	    {     
 	      struct thread_info *ti = inferior_thread ();
+	      struct cleanup *cleanup;
 
 	      target_terminal_ours ();
-	      fprintf_unfiltered (mi->event_channel, 
-				  "thread-selected,id=\"%d\"",
-				  ti->num);
-	      gdb_flush (mi->event_channel);
+
+	      cleanup
+		= make_cleanup_ui_out_list_begin_end (mi->event_channel,
+						      "thread-selected");
+	      ui_out_field_int (mi->event_channel, "id", ti->num);
+	      do_cleanups (cleanup);
 	    }
 	}
 
@@ -2192,11 +2195,11 @@ mi_load_progress (const char *section_name,
 
   if (current_interp_named_p (INTERP_MI)
       || current_interp_named_p (INTERP_MI2))
-    uiout = mi_out_new (2);
+    uiout = mi_out_new (2, NULL);
   else if (current_interp_named_p (INTERP_MI1))
-    uiout = mi_out_new (1);
+    uiout = mi_out_new (1, NULL);
   else if (current_interp_named_p (INTERP_MI3))
-    uiout = mi_out_new (3);
+    uiout = mi_out_new (3, NULL);
   else
     return;
 
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 9aaeec6..4cd6abc 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -29,6 +29,12 @@ struct ui_out_data
     int suppress_field_separator;
     int suppress_output;
     int mi_version;
+    /* True if this ui-out is for event notification.  This changes
+       how some lists are output.  */
+    int for_events;
+    /* The number of list-opens we have seen.  This is only used when
+       FOR_EVENTS is true.  */
+    int list_depth;
     struct ui_file *buffer;
   };
 typedef struct ui_out_data mi_out_data;
@@ -315,18 +321,34 @@ mi_open (struct ui_out *uiout,
 	 enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  int event_start = 0;
 
-  field_separator (uiout);
-  data->suppress_field_separator = 1;
+  if (type == ui_out_type_list && data->for_events)
+    {
+      ++data->list_depth;
+      if (data->list_depth == 1)
+	event_start = 1;
+    }
+
+  if (!event_start)
+    {
+      field_separator (uiout);
+      data->suppress_field_separator = 1;
+    }
   if (name)
-    fprintf_unfiltered (data->buffer, "%s=", name);
+    {
+      fputs_unfiltered (name, data->buffer);
+      if (!event_start)
+	fputs_unfiltered ("=", data->buffer);
+    }
   switch (type)
     {
     case ui_out_type_tuple:
       fputc_unfiltered ('{', data->buffer);
       break;
     case ui_out_type_list:
-      fputc_unfiltered ('[', data->buffer);
+      if (!event_start)
+	fputc_unfiltered ('[', data->buffer);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -338,6 +360,14 @@ mi_close (struct ui_out *uiout,
 	  enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  int event_end = 0;
+
+  if (type == ui_out_type_list && data->for_events)
+    {
+      --data->list_depth;
+      if (data->list_depth == 0)
+	event_end = 1;
+    }
 
   switch (type)
     {
@@ -345,12 +375,16 @@ mi_close (struct ui_out *uiout,
       fputc_unfiltered ('}', data->buffer);
       break;
     case ui_out_type_list:
-      fputc_unfiltered (']', data->buffer);
+      if (!event_end)
+	fputc_unfiltered (']', data->buffer);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
     }
   data->suppress_field_separator = 0;
+
+  if (event_end)
+    mi_flush (uiout);
 }
 
 /* add a string to the buffer */
@@ -401,10 +435,10 @@ mi_version (struct ui_out *uiout)
   return data->mi_version;
 }
 
-/* initalize private members at startup */
+/* Initialize private members at startup.  */
 
 struct ui_out *
-mi_out_new (int mi_version)
+mi_out_new (int mi_version, struct ui_file *out)
 {
   int flags = 0;
 
@@ -412,9 +446,14 @@ mi_out_new (int mi_version)
   data->suppress_field_separator = 0;
   data->suppress_output = 0;
   data->mi_version = mi_version;
+  data->for_events = out != NULL;
+  data->list_depth = 0;
   /* FIXME: This code should be using a ``string_file'' and not the
      TUI buffer hack. */
-  data->buffer = mem_fileopen ();
+  if (out)
+    data->buffer = out;
+  else
+    data->buffer = mem_fileopen ();
   return ui_out_new (&mi_ui_out_impl, data, flags);
 }
 
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index 515aefa..d7bf0a6 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -24,7 +24,7 @@
 struct ui_out;
 struct ui_file;
 
-extern struct ui_out *mi_out_new (int mi_version);
+extern struct ui_out *mi_out_new (int mi_version, struct ui_file *stream);
 extern void mi_out_put (struct ui_out *uiout, struct ui_file *stream);
 extern void mi_out_rewind (struct ui_out *uiout);
 extern void mi_out_buffered (struct ui_out *uiout, char *string);