Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* cleanup mi error message handling
@ 2008-03-24 18:30 Pedro Alves
  2008-03-24 19:08 ` Pedro Alves
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Pedro Alves @ 2008-03-24 18:30 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

I was looking through cleaning up mi/ global state, and noticed
the global mi_error_message used to output ^error,msg=""
messages.

I then noticed that the errors output by the mi/ module are
also duplicated to stderr.

>./gdb -i=mi
~"GNU gdb 6.8.50.20080323-cvs\n"
~"Copyright (C) 2008 Free Software Foundation, Inc.\n"
~"License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>\n"
~"This is free software: you are free to change and redistribute it.\n"
~"There is NO WARRANTY, to the extent permitted by law.  Type \"show 
copying\"\n"
~"and \"show warranty\" for details.\n"
~"This GDB was configured as \"x86_64-unknown-linux-gnu\".\n"
~"Setting up the environment for debugging gdb.\n"
&"/home/pedro/gdb/head/build/gdb/.gdbinit:5: Error in sourced command file:\n"
&"No symbol table is loaded.  Use the \"file\" command.\n"
(gdb)
-exec-continue
^running
(gdb)
&"The program is not being run.\n"
^error,msg="The program is not being run."
(gdb)


Notice the duplication:
&"The program is not being run.\n"
^error,msg="The program is not being run."

I can't find a valid reason for that.
In fact having them display in gdb's output window in a
frontend doesn't help anyone, especially, since ^error is supposed
to be the error output channel.  Furthermore, there are a couple
of cases that weren't using the mechanism correctly, as they
were already calling error.

The attached patch fixes it, and fixes the testsuite to not
expect this duplication.

-- 
Pedro Alves

[-- Attachment #2: mi_exceptions.diff --]
[-- Type: text/x-diff, Size: 46241 bytes --]

gdb/
2008-03-24  Pedro Alves  <pedro@codesourcery.com>

	* mi/mi-cmds.h (enum mi_cmd_result): Delete MI_CMD_ERROR.
	(mi_error_message): Delete declaration.
	* mi/mi-interp.c (mi_cmd_interpreter_exec): Call error instead of
	returning MI_CMD_ERROR.
	* mi/mi-main.c (mi_error_message): Delete.
	(mi_cmd_exec_interrupt):
	(mi_cmd_thread_select, mi_cmd_thread_list_ids)
	(mi_cmd_thread_info): Call error instead of returning
	MI_CMD_ERROR.
	(mi_cmd_data_list_register_values): Call error instead of
	returning MI_CMD_ERROR.  Adapt to new get_register interface.
	(get_register): Change return typo to void.  Call error instead of
	returning MI_CMD_ERROR.
	(mi_cmd_data_write_register_values): Call error instead of
	returning MI_CMD_ERROR.
	(mi_cmd_list_features): Return MI_CMD_DONE.
	(captured_mi_execute_command): Remove MI_CMD_ERROR handling.
	(mi_execute_command): Always print exceptions with -error.
	

gdb/testsuite/
2008-03-24  Pedro Alves  <pedro@codesourcery.com>

	* gdb.mi/mi-disassemble.exp, gdb.mi/mi-stack.exp,
	gdb.mi/mi-syn-frame.exp, gdb.mi/mi-var-block.exp,
	gdb.mi/mi-var-cmd.exp, gdb.mi/mi-var-display.exp,
	gdb.mi/mi2-disassemble.exp, gdb.mi/mi2-stack.exp,
	gdb.mi/mi2-syn-frame.exp, gdb.mi/mi2-var-block.exp,
	gdb.mi/mi2-var-cmd.exp, gdb.mi/mi2-var-display.exp: Update to not
	expect an mi error duplicated in stderr.

---
 gdb/mi/mi-cmds.h                         |    5 
 gdb/mi/mi-interp.c                       |   24 --
 gdb/mi/mi-main.c                         |  251 +++++++++----------------------
 gdb/testsuite/gdb.mi/mi-disassemble.exp  |    8 
 gdb/testsuite/gdb.mi/mi-stack.exp        |    8 
 gdb/testsuite/gdb.mi/mi-syn-frame.exp    |    7 
 gdb/testsuite/gdb.mi/mi-var-block.exp    |    4 
 gdb/testsuite/gdb.mi/mi-var-cmd.exp      |   10 -
 gdb/testsuite/gdb.mi/mi-var-display.exp  |    2 
 gdb/testsuite/gdb.mi/mi2-disassemble.exp |    8 
 gdb/testsuite/gdb.mi/mi2-stack.exp       |    8 
 gdb/testsuite/gdb.mi/mi2-syn-frame.exp   |    6 
 gdb/testsuite/gdb.mi/mi2-var-block.exp   |    4 
 gdb/testsuite/gdb.mi/mi2-var-cmd.exp     |   10 -
 gdb/testsuite/gdb.mi/mi2-var-display.exp |    2 
 15 files changed, 126 insertions(+), 231 deletions(-)

Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/mi/mi-main.c	2008-03-23 20:00:02.000000000 +0000
@@ -96,7 +96,6 @@ static int do_timings = 0;
 /* The token of the last asynchronous command.  */
 static char *last_async_command;
 static char *previous_async_command;
-char *mi_error_message;
 
 extern void _initialize_mi_main (void);
 static enum mi_cmd_result mi_cmd_execute (struct mi_parse *parse);
@@ -109,7 +108,7 @@ static void mi_exec_async_cli_cmd_contin
 
 static int register_changed_p (int regnum, struct regcache *,
 			       struct regcache *);
-static int get_register (int regnum, int format);
+static void get_register (int regnum, int format);
 
 /* Command implementations.  FIXME: Is this libgdb?  No.  This is the MI
    layer that calls libgdb.  Any operation used in the below should be
@@ -219,10 +218,8 @@ enum mi_cmd_result
 mi_cmd_exec_interrupt (char *args, int from_tty)
 {
   if (!target_executing)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_exec_interrupt: Inferior not executing.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_exec_interrupt: Inferior not executing.");
+
   interrupt_target_command (args, from_tty);
   if (last_async_command)
     fputs_unfiltered (last_async_command, raw_stdout);
@@ -242,38 +239,40 @@ enum mi_cmd_result
 mi_cmd_thread_select (char *command, char **argv, int argc)
 {
   enum gdb_rc rc;
+  char *mi_error_message;
 
   if (argc != 1)
+    error ("mi_cmd_thread_select: USAGE: threadnum.");
+
+  rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
+
+  if (rc == GDB_RC_FAIL)
     {
-      mi_error_message = xstrprintf ("mi_cmd_thread_select: USAGE: threadnum.");
-      return MI_CMD_ERROR;
+      make_cleanup (xfree, mi_error_message);
+      error ("%s", mi_error_message);
     }
-  else
-    rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
 
-  if (rc == GDB_RC_FAIL)
-    return MI_CMD_ERROR;
-  else
-    return MI_CMD_DONE;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
 mi_cmd_thread_list_ids (char *command, char **argv, int argc)
 {
   enum gdb_rc rc;
+  char *mi_error_message;
 
   if (argc != 0)
+    error ("mi_cmd_thread_list_ids: No arguments required.");
+
+  rc = gdb_list_thread_ids (uiout, &mi_error_message);
+
+  if (rc == GDB_RC_FAIL)
     {
-      mi_error_message = xstrprintf ("mi_cmd_thread_list_ids: No arguments required.");
-      return MI_CMD_ERROR;
+      make_cleanup (xfree, mi_error_message);
+      error ("%s", mi_error_message);
     }
-  else
-    rc = gdb_list_thread_ids (uiout, &mi_error_message);
 
-  if (rc == GDB_RC_FAIL)
-    return MI_CMD_ERROR;
-  else
-    return MI_CMD_DONE;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
@@ -282,10 +281,7 @@ mi_cmd_thread_info (char *command, char 
   int thread = -1;
   
   if (argc != 0 && argc != 1)
-    {
-      mi_error_message = xstrprintf ("Invalid MI command");
-      return MI_CMD_ERROR;
-    }
+    error ("Invalid MI command");
 
   if (argc == 1)
     thread = atoi (argv[0]);
@@ -333,11 +329,8 @@ mi_cmd_data_list_register_names (char *c
     {
       regnum = atoi (argv[i]);
       if (regnum < 0 || regnum >= numregs)
-	{
-	  do_cleanups (cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
+
       if (gdbarch_register_name (current_gdbarch, regnum) == NULL
 	  || *(gdbarch_register_name (current_gdbarch, regnum)) == '\0')
 	ui_out_field_string (uiout, NULL, "");
@@ -388,11 +381,7 @@ mi_cmd_data_list_changed_registers (char
 	    continue;
 	  changed = register_changed_p (regnum, prev_regs, this_regs);
 	  if (changed < 0)
-	    {
-	      do_cleanups (cleanup);
-	      mi_error_message = xstrprintf ("mi_cmd_data_list_changed_registers: Unable to read register contents.");
-	      return MI_CMD_ERROR;
-	    }
+	    error ("mi_cmd_data_list_changed_registers: Unable to read register contents.");
 	  else if (changed)
 	    ui_out_field_int (uiout, NULL, regnum);
 	}
@@ -410,20 +399,12 @@ mi_cmd_data_list_changed_registers (char
 	{
 	  changed = register_changed_p (regnum, prev_regs, this_regs);
 	  if (changed < 0)
-	    {
-	      do_cleanups (cleanup);
-	      mi_error_message = xstrprintf ("mi_cmd_data_list_register_change: Unable to read register contents.");
-	      return MI_CMD_ERROR;
-	    }
+	    error ("mi_cmd_data_list_register_change: Unable to read register contents.");
 	  else if (changed)
 	    ui_out_field_int (uiout, NULL, regnum);
 	}
       else
-	{
-	  do_cleanups (cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   do_cleanups (cleanup);
   return MI_CMD_DONE;
@@ -465,7 +446,7 @@ register_changed_p (int regnum, struct r
 enum mi_cmd_result
 mi_cmd_data_list_register_values (char *command, char **argv, int argc)
 {
-  int regnum, numregs, format, result;
+  int regnum, numregs, format;
   int i;
   struct cleanup *list_cleanup, *tuple_cleanup;
 
@@ -479,10 +460,7 @@ mi_cmd_data_list_register_values (char *
 	    + gdbarch_num_pseudo_regs (current_gdbarch);
 
   if (argc == 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: Usage: -data-list-register-values <format> [<regnum1>...<regnumN>]");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_list_register_values: Usage: -data-list-register-values <format> [<regnum1>...<regnumN>]");
 
   format = (int) argv[0][0];
 
@@ -499,12 +477,7 @@ mi_cmd_data_list_register_values (char *
 	    continue;
 	  tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_int (uiout, "number", regnum);
-	  result = get_register (regnum, format);
-	  if (result == -1)
-	    {
-	      do_cleanups (list_cleanup);
-	      return MI_CMD_ERROR;
-	    }
+	  get_register (regnum, format);
 	  do_cleanups (tuple_cleanup);
 	}
     }
@@ -521,27 +494,18 @@ mi_cmd_data_list_register_values (char *
 	{
 	  tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_int (uiout, "number", regnum);
-	  result = get_register (regnum, format);
-	  if (result == -1)
-	    {
-	      do_cleanups (list_cleanup);
-	      return MI_CMD_ERROR;
-	    }
+	  get_register (regnum, format);
 	  do_cleanups (tuple_cleanup);
 	}
       else
-	{
-	  do_cleanups (list_cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   do_cleanups (list_cleanup);
   return MI_CMD_DONE;
 }
 
 /* Output one register's contents in the desired format.  */
-static int
+static void
 get_register (int regnum, int format)
 {
   gdb_byte buffer[MAX_REGISTER_SIZE];
@@ -560,10 +524,7 @@ get_register (int regnum, int format)
 		  &realnum, buffer);
 
   if (optim)
-    {
-      mi_error_message = xstrprintf ("Optimized out");
-      return -1;
-    }
+    error ("Optimized out");
 
   if (format == 'r')
     {
@@ -589,7 +550,6 @@ get_register (int regnum, int format)
       ui_out_field_stream (uiout, "value", stb);
       ui_out_stream_delete (stb);
     }
-  return 1;
 }
 
 /* Write given values into registers. The registers and values are
@@ -611,30 +571,18 @@ mi_cmd_data_write_register_values (char 
 	    + gdbarch_num_pseudo_regs (current_gdbarch);
 
   if (argc == 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: Usage: -data-write-register-values <format> [<regnum1> <value1>...<regnumN> <valueN>]");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: Usage: -data-write-register-values <format> [<regnum1> <value1>...<regnumN> <valueN>]");
 
   format = (int) argv[0][0];
 
   if (!target_has_registers)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: No registers.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: No registers.");
 
   if (!(argc - 1))
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: No regs and values specified.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: No regs and values specified.");
 
   if ((argc - 1) % 2)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: Regs and vals are not in pairs.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: Regs and vals are not in pairs.");
 
   for (i = 1; i < argc; i = i + 2)
     {
@@ -653,10 +601,7 @@ mi_cmd_data_write_register_values (char 
 	  regcache_cooked_write_signed (get_current_regcache (), regnum, value);
 	}
       else
-	{
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   return MI_CMD_DONE;
 }
@@ -676,9 +621,8 @@ mi_cmd_data_evaluate_expression (char *c
 
   if (argc != 1)
     {
-      mi_error_message = xstrprintf ("mi_cmd_data_evaluate_expression: Usage: -data-evaluate-expression expression");
       ui_out_stream_delete (stb);
-      return MI_CMD_ERROR;
+      error ("mi_cmd_data_evaluate_expression: Usage: -data-evaluate-expression expression");
     }
 
   expr = parse_expression (argv[0]);
@@ -808,10 +752,7 @@ mi_cmd_data_read_memory (char *command, 
   argc -= optind;
 
   if (argc < 5 || argc > 6)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: Usage: ADDR WORD-FORMAT WORD-SIZE NR-ROWS NR-COLS [ASCHAR].");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: Usage: ADDR WORD-FORMAT WORD-SIZE NR-ROWS NR-COLS [ASCHAR].");
 
   /* Extract all the arguments. */
 
@@ -847,17 +788,13 @@ mi_cmd_data_read_memory (char *command, 
   /* The number of rows.  */
   nr_rows = atol (argv[3]);
   if (nr_rows <= 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: invalid number of rows.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: invalid number of rows.");
+
   /* Number of bytes per row.  */
   nr_cols = atol (argv[4]);
   if (nr_cols <= 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: invalid number of columns.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: invalid number of columns.");
+
   /* The un-printable character when printing ascii.  */
   if (argc == 6)
     aschar = *argv[5];
@@ -872,11 +809,7 @@ mi_cmd_data_read_memory (char *command, 
   nr_bytes = target_read (&current_target, TARGET_OBJECT_MEMORY, NULL,
 			  mbuf, addr, total_bytes);
   if (nr_bytes <= 0)
-    {
-      do_cleanups (cleanups);
-      mi_error_message = xstrdup ("Unable to read memory.");
-      return MI_CMD_ERROR;
-    }
+    error ("Unable to read memory.");
 
   /* Output the header information.  */
   ui_out_field_core_addr (uiout, "addr", addr);
@@ -1008,10 +941,7 @@ mi_cmd_data_write_memory (char *command,
   argc -= optind;
 
   if (argc != 4)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_memory: Usage: [-o COLUMN_OFFSET] ADDR FORMAT WORD-SIZE VALUE.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_memory: Usage: [-o COLUMN_OFFSET] ADDR FORMAT WORD-SIZE VALUE.");
 
   /* Extract all the arguments.  */
   /* Start address of the memory dump.  */
@@ -1060,7 +990,7 @@ mi_cmd_enable_timings (char *command, ch
 
  usage_error:
   error ("mi_cmd_enable_timings: Usage: %s {yes|no}", command);
-  return MI_CMD_ERROR;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
@@ -1081,7 +1011,7 @@ mi_cmd_list_features (char *command, cha
     }
 
   error ("-list-features should be passed no arguments");
-  return MI_CMD_ERROR;
+  return MI_CMD_DONE;
 }
  
 /* Execute a command within a safe environment.
@@ -1121,7 +1051,7 @@ captured_mi_execute_command (struct ui_o
       args->rc = mi_cmd_execute (context);
 
       if (do_timings)
-          timestamp (&cmd_finished);
+	timestamp (&cmd_finished);
 
       if (!target_can_async_p () || !target_executing)
 	{
@@ -1143,19 +1073,6 @@ captured_mi_execute_command (struct ui_o
 		  print_diff (context->cmd_start, &cmd_finished);
 	      fputs_unfiltered ("\n", raw_stdout);
 	    }
-	  else if (args->rc == MI_CMD_ERROR)
-	    {
-	      if (mi_error_message)
-		{
-		  fputs_unfiltered (context->token, raw_stdout);
-		  fputs_unfiltered ("^error,msg=\"", raw_stdout);
-		  fputstr_unfiltered (mi_error_message, '"', raw_stdout);
-		  xfree (mi_error_message);
-		  mi_error_message = NULL;
-		  fputs_unfiltered ("\"\n", raw_stdout);
-		}
-	      mi_out_rewind (uiout);
-	    }
 	  else
 	    mi_out_rewind (uiout);
 	}
@@ -1196,19 +1113,6 @@ captured_mi_execute_command (struct ui_o
 		fputs_unfiltered ("\n", raw_stdout);
 		args->action = EXECUTE_COMMAND_DISPLAY_PROMPT;
 	      }
-	    else if (args->rc == MI_CMD_ERROR)
-	      {
-		if (mi_error_message)
-		  {
-		    fputs_unfiltered (context->token, raw_stdout);
-		    fputs_unfiltered ("^error,msg=\"", raw_stdout);
-		    fputstr_unfiltered (mi_error_message, '"', raw_stdout);
-		    xfree (mi_error_message);
-		    mi_error_message = NULL;
-		    fputs_unfiltered ("\"\n", raw_stdout);
-		  }
-		mi_out_rewind (uiout);
-	      }
 	    else
 	      mi_out_rewind (uiout);
 	  }
@@ -1246,20 +1150,9 @@ mi_execute_command (char *cmd, int from_
 	  timestamp (command->cmd_start);
 	}
 
-      /* FIXME: cagney/1999-11-04: Can this use of catch_exceptions either
-         be pushed even further down or even eliminated?  */
       args.command = command;
       result = catch_exception (uiout, captured_mi_execute_command, &args,
 				RETURN_MASK_ALL);
-      exception_print (gdb_stderr, result);
-
-      if (args.action == EXECUTE_COMMAND_SUPRESS_PROMPT)
-	{
-	  /* The command is executing synchronously.  Bail out early
-	     suppressing the finished prompt.  */
-	  mi_parse_free (command);
-	  return;
-	}
       if (result.reason < 0)
 	{
 	  /* The command execution failed and error() was called
@@ -1269,11 +1162,17 @@ mi_execute_command (char *cmd, int from_
 	  if (result.message == NULL)
 	    fputs_unfiltered ("unknown error", raw_stdout);
 	  else
-	      fputstr_unfiltered (result.message, '"', raw_stdout);
+	    fputstr_unfiltered (result.message, '"', raw_stdout);
 	  fputs_unfiltered ("\"\n", raw_stdout);
 	  mi_out_rewind (uiout);
 	}
+
       mi_parse_free (command);
+
+      if (args.action == EXECUTE_COMMAND_SUPRESS_PROMPT)
+	/* The command is executing synchronously.  Bail out early
+	   suppressing the finished prompt.  */
+	return;
     }
 
   fputs_unfiltered ("(gdb) \n", raw_stdout);
@@ -1311,13 +1210,15 @@ mi_cmd_execute (struct mi_parse *parse)
 	    previous_async_command = xstrdup (last_async_command);
 	  if (strcmp (parse->command, "exec-interrupt"))
 	    {
-	      fputs_unfiltered (parse->token, raw_stdout);
-	      fputs_unfiltered ("^error,msg=\"", raw_stdout);
-	      fputs_unfiltered ("Cannot execute command ", raw_stdout);
-	      fputstr_unfiltered (parse->command, '"', raw_stdout);
-	      fputs_unfiltered (" while target running", raw_stdout);
-	      fputs_unfiltered ("\"\n", raw_stdout);
-	      return MI_CMD_ERROR;
+	      struct ui_file *stb;
+	      stb = mem_fileopen ();
+
+	      fputs_unfiltered ("Cannot execute command ", stb);
+	      fputstr_unfiltered (parse->command, '"', stb);
+	      fputs_unfiltered (" while target running", stb);
+
+	      make_cleanup_ui_file_delete (stb);
+	      error_stream (stb);
 	    }
 	}
       last_async_command = xstrdup (parse->token);
@@ -1339,13 +1240,19 @@ mi_cmd_execute (struct mi_parse *parse)
   else
     {
       /* FIXME: DELETE THIS.  */
-      fputs_unfiltered (parse->token, raw_stdout);
-      fputs_unfiltered ("^error,msg=\"", raw_stdout);
-      fputs_unfiltered ("Undefined mi command: ", raw_stdout);
-      fputstr_unfiltered (parse->command, '"', raw_stdout);
-      fputs_unfiltered (" (missing implementation)", raw_stdout);
-      fputs_unfiltered ("\"\n", raw_stdout);
-      return MI_CMD_ERROR;
+      struct ui_file *stb;
+
+      stb = mem_fileopen ();
+
+      fputs_unfiltered ("Undefined mi command: ", stb);
+      fputstr_unfiltered (parse->command, '"', stb);
+      fputs_unfiltered (" (missing implementation)", stb);
+
+      make_cleanup_ui_file_delete (stb);
+      error_stream (stb);
+
+      /* unreacheable */
+      return MI_CMD_DONE;
     }
 }
 
Index: src/gdb/mi/mi-cmds.h
===================================================================
--- src.orig/gdb/mi/mi-cmds.h	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/mi/mi-cmds.h	2008-03-23 19:56:36.000000000 +0000
@@ -33,10 +33,6 @@ enum mi_cmd_result
     /* The command is still running in the forground.  Main loop should
        display the completion prompt. */
     MI_CMD_FORGROUND,
-    /* An error condition was detected and an error message was
-       asprintf'd into the mi_error_message buffer.  The main loop will
-       display the error message and the completion prompt. */
-    MI_CMD_ERROR,
     /* The MI command has already displayed its completion message.
        Main loop will not display a completion message but will display
        the completion prompt. */
@@ -156,7 +152,6 @@ extern int mi_debug_p;
 /* Raw console output - FIXME: should this be a parameter? */
 extern struct ui_file *raw_stdout;
 
-extern char *mi_error_message;
 extern void mi_execute_command (char *cmd, int from_tty);
 
 #endif
Index: src/gdb/mi/mi-interp.c
===================================================================
--- src.orig/gdb/mi/mi-interp.c	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/mi/mi-interp.c	2008-03-23 19:56:36.000000000 +0000
@@ -190,29 +190,20 @@ enum mi_cmd_result
 mi_cmd_interpreter_exec (char *command, char **argv, int argc)
 {
   struct interp *interp_to_use;
-  enum mi_cmd_result result = MI_CMD_DONE;
   int i;
   struct interp_procs *procs;
+  char *mi_error_message = NULL;
 
   if (argc < 2)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command");
 
   interp_to_use = interp_lookup (argv[0]);
   if (interp_to_use == NULL)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: could not find interpreter \"%s\"", argv[0]);
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: could not find interpreter \"%s\"", argv[0]);
 
   if (!interp_exec_p (interp_to_use))
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: interpreter \"%s\" does not support command execution",
-				     argv[0]);
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: interpreter \"%s\" does not support command execution",
+	      argv[0]);
 
   /* Insert the MI out hooks, making sure to also call the interpreter's hooks
      if it has any. */
@@ -228,7 +219,6 @@ mi_cmd_interpreter_exec (char *command, 
       if (e.reason < 0)
 	{
 	  mi_error_message = xstrdup (e.message);
-	  result = MI_CMD_ERROR;
 	  break;
 	}
     }
@@ -246,7 +236,9 @@ mi_cmd_interpreter_exec (char *command, 
       add_continuation (mi_interpreter_exec_continuation, NULL);
     }
 
-  return result;
+  if (mi_error_message != NULL)
+    error ("%s", mi_error_message);
+  return MI_CMD_DONE;
 }
 
 /*
Index: src/gdb/testsuite/gdb.mi/mi-disassemble.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-disassemble.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-disassemble.exp	2008-03-23 19:56:36.000000000 +0000
@@ -160,19 +160,19 @@ proc test_disassembly_bogus_args {} {
     # -data-disassembly -f basics.c -l 32 -- 9
 
     mi_gdb_test "123-data-disassemble -f foo -l abc -n 0 -- 0" \
-             "&.*123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
+             "123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
              "data-disassemble bogus filename"
 
     mi_gdb_test "321-data-disassemble -s foo -e bar -- 0" \
-             "&.*321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
+             "321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
              "data-disassemble bogus address"
 
     mi_gdb_test "456-data-disassemble -s \$pc -f basics.c -- 0" \
-             "&.*456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
+             "456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
              "data-disassemble mix different args"
 
     mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
-             "&.*789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
+             "789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
              "data-disassemble wrong mode arg"
 
 }
Index: src/gdb/testsuite/gdb.mi/mi-stack.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-stack.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-stack.exp	2008-03-23 19:56:36.000000000 +0000
@@ -69,7 +69,7 @@ proc test_stack_frame_listing {} {
                 "stack frame listing 1 3"
 
     mi_gdb_test "234-stack-list-frames 1" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack frame listing wrong"
 
     mi_gdb_test "235-stack-info-frame" \
@@ -120,7 +120,7 @@ proc test_stack_args_listing {} {
                 "stack args listing 1 1 3"
 
     mi_gdb_test "234-stack-list-arguments" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack args listing wrong"
 
     mi_gdb_test "235-stack-list-arguments 1 1 300" \
@@ -151,7 +151,7 @@ proc test_stack_info_depth {} {
                 "stack info-depth 99"
 
     mi_gdb_test "231-stack-info-depth 99 99" \
-	    "&.*231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
+	    "231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
                 "stack info-depth wrong usage"
 }
 
@@ -190,7 +190,7 @@ gdb_expect {
   "stack locals listing, simple types: names and values, complex type: names and types"
 
     mi_gdb_test "234-stack-list-locals" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
 	    "stack locals listing wrong"
 
     mi_gdb_test "232-stack-select-frame 1" \
Index: src/gdb/testsuite/gdb.mi/mi-var-block.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-block.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-var-block.exp	2008-03-23 19:56:36.000000000 +0000
@@ -49,7 +49,7 @@ mi_gdb_test "-var-create cb * cb" \
 	"create local variable cb"
 
 mi_gdb_test "-var-create foo * foo" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create local variable foo"
 
 # step to "foo = 123;"
@@ -60,7 +60,7 @@ mi_step_to "do_block_tests" "" "var-cmd.
 
 # Be paranoid and assume 3.2 created foo
 mi_gdb_test "-var-delete foo" \
-	"&\"Variable object not found\\\\n\".*\\^error,msg=\"Variable object not found\"" \
+	"\\^error,msg=\"Variable object not found\"" \
 	"delete var foo"
 
 
Index: src/gdb/testsuite/gdb.mi/mi-var-cmd.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-cmd.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-var-cmd.exp	2008-03-23 19:56:36.000000000 +0000
@@ -58,14 +58,14 @@ mi_gdb_test "111-var-create global_simpl
 # Desc: Create non-existent variable
 
 mi_gdb_test "112-var-create bogus_unknown_variable * bogus_unknown_variable" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create non-existent variable"
 
 # Test: c_variable-1.3
 # Desc: Create out of scope variable
 
 mi_gdb_test "113-var-create argc * argc" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create out of scope variable"
 
 mi_runto do_locals_tests
@@ -154,7 +154,7 @@ mi_gdb_test "-var-create lsimple.integer
 #    Type names (like int, long, etc..) are all proper expressions to gdb.
 #    make sure variable code does not allow users to create variables, though.
 mi_gdb_test "-var-create int * int" \
-	"&\"Attempt to use a type name as an expression.\\\\n\".*&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"&\"Attempt to use a type name as an expression.\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create int"
 
 
@@ -275,7 +275,7 @@ mi_gdb_test "-var-update *" \
 #
 ###
 mi_gdb_test "-var-assign global_simple 0" \
-	"&\"mi_cmd_var_assign: Variable object is not editable\\\\n\".*\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
+	"\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
 	"assign to global_simple"
 
 mi_gdb_test "-var-assign linteger 3333" \
@@ -449,7 +449,7 @@ mi_gdb_test "-var-create l * l" \
 # Test: c_variable-2.11
 # Desc: create do_locals_tests local in subroutine1
 mi_gdb_test "-var-create linteger * linteger" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create linteger"
 
 mi_step_to "subroutine1" "\{name=\"i\",value=\".*\"\},\{name=\"l\",value=\".*\"\}" \
Index: src/gdb/testsuite/gdb.mi/mi-var-display.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-display.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-var-display.exp	2008-03-23 19:56:36.000000000 +0000
@@ -544,7 +544,7 @@ clear_xfail "*-*-*"
 # Test: c_variable-7.70
 # Desc: create anone
 mi_gdb_test "-var-create anone * anone" \
-	"&\"Duplicate variable object name\\\\n\".*\\^error,msg=\"Duplicate variable object name\"" \
+	"\\^error,msg=\"Duplicate variable object name\"" \
 	"create duplicate local variable anone"
 
 
Index: src/gdb/testsuite/gdb.mi/mi2-disassemble.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-disassemble.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-disassemble.exp	2008-03-23 19:56:36.000000000 +0000
@@ -160,19 +160,19 @@ proc test_disassembly_bogus_args {} {
     # -data-disassembly -f basics.c -l 32 -- 9
 
     mi_gdb_test "123-data-disassemble -f foo -l abc -n 0 -- 0" \
-             "&.*123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
+             "123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
              "data-disassemble bogus filename"
 
     mi_gdb_test "321-data-disassemble -s foo -e bar -- 0" \
-             "&.*321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
+             "321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
              "data-disassemble bogus address"
 
     mi_gdb_test "456-data-disassemble -s \$pc -f basics.c -- 0" \
-             "&.*456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
+             "456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
              "data-disassemble mix different args"
 
     mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
-             "&.*789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
+             "789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
              "data-disassemble wrong mode arg"
 
 }
Index: src/gdb/testsuite/gdb.mi/mi2-stack.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-stack.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-stack.exp	2008-03-23 19:56:36.000000000 +0000
@@ -69,7 +69,7 @@ proc test_stack_frame_listing {} {
                 "stack frame listing 1 3"
 
     mi_gdb_test "234-stack-list-frames 1" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack frame listing wrong"
 
     mi_gdb_test "235-stack-info-frame" \
@@ -120,7 +120,7 @@ proc test_stack_args_listing {} {
                 "stack args listing 1 1 3"
 
     mi_gdb_test "234-stack-list-arguments" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack args listing wrong"
 
     mi_gdb_test "235-stack-list-arguments 1 1 300" \
@@ -151,7 +151,7 @@ proc test_stack_info_depth {} {
                 "stack info-depth 99"
 
     mi_gdb_test "231-stack-info-depth 99 99" \
-	    "&.*231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
+	    "231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
                 "stack info-depth wrong usage"
 }
 
@@ -190,7 +190,7 @@ gdb_expect {
   "stack locals listing, simple types: names and values, complex type: names and types"
 
     mi_gdb_test "234-stack-list-locals" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
 	    "stack locals listing wrong"
 
     mi_gdb_test "232-stack-select-frame 1" \
Index: src/gdb/testsuite/gdb.mi/mi2-var-block.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-block.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-var-block.exp	2008-03-23 19:56:36.000000000 +0000
@@ -49,7 +49,7 @@ mi_gdb_test "-var-create cb * cb" \
 	"create local variable cb"
 
 mi_gdb_test "-var-create foo * foo" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create local variable foo"
 
 # step to "foo = 123;"
@@ -60,7 +60,7 @@ mi_step_to "do_block_tests" "" "var-cmd.
 
 # Be paranoid and assume 3.2 created foo
 mi_gdb_test "-var-delete foo" \
-	"&\"Variable object not found\\\\n\".*\\^error,msg=\"Variable object not found\"" \
+	"\\^error,msg=\"Variable object not found\"" \
 	"delete var foo"
 
 
Index: src/gdb/testsuite/gdb.mi/mi2-var-cmd.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-cmd.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-var-cmd.exp	2008-03-23 19:56:36.000000000 +0000
@@ -58,14 +58,14 @@ mi_gdb_test "111-var-create global_simpl
 # Desc: Create non-existent variable
 
 mi_gdb_test "112-var-create bogus_unknown_variable * bogus_unknown_variable" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create non-existent variable"
 
 # Test: c_variable-1.3
 # Desc: Create out of scope variable
 
 mi_gdb_test "113-var-create argc * argc" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create out of scope variable"
 
 mi_runto do_locals_tests
@@ -154,7 +154,7 @@ mi_gdb_test "-var-create lsimple.integer
 #    Type names (like int, long, etc..) are all proper expressions to gdb.
 #    make sure variable code does not allow users to create variables, though.
 mi_gdb_test "-var-create int * int" \
-	"&\"Attempt to use a type name as an expression.\\\\n\".*&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"&\"Attempt to use a type name as an expression.\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create int"
 
 
@@ -275,7 +275,7 @@ mi_gdb_test "-var-update *" \
 #
 ###
 mi_gdb_test "-var-assign global_simple 0" \
-	"&\"mi_cmd_var_assign: Variable object is not editable\\\\n\".*\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
+	"\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
 	"assign to global_simple"
 
 mi_gdb_test "-var-assign linteger 3333" \
@@ -412,7 +412,7 @@ mi_gdb_test "-var-create l * l" \
 # Test: c_variable-2.11
 # Desc: create do_locals_tests local in subroutine1
 mi_gdb_test "-var-create linteger * linteger" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create linteger"
 
 mi_step_to "subroutine1" "\{name=\"i\",value=\".*\"\},\{name=\"l\",value=\".*\"\}" \
Index: src/gdb/testsuite/gdb.mi/mi2-var-display.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-display.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-var-display.exp	2008-03-23 19:56:36.000000000 +0000
@@ -544,7 +544,7 @@ clear_xfail "*-*-*"
 # Test: c_variable-7.70
 # Desc: create anone
 mi_gdb_test "-var-create anone * anone" \
-	"&\"Duplicate variable object name\\\\n\".*\\^error,msg=\"Duplicate variable object name\"" \
+	"\\^error,msg=\"Duplicate variable object name\"" \
 	"create duplicate local variable anone"
 
 
Index: src/gdb/testsuite/gdb.mi/mi-syn-frame.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-syn-frame.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-syn-frame.exp	2008-03-23 19:56:36.000000000 +0000
@@ -48,7 +48,8 @@ mi_gdb_test "400-break-insert foo" \
 # Call foo() by hand, where we'll hit a breakpoint.
 #
 
-mi_gdb_test "401-data-evaluate-expression foo()" "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(foo\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.*\"" "call inferior's function with a breakpoint set in it"
+mi_gdb_test "401-data-evaluate-expression foo()" "401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(foo\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" "call inferior's function with a breakpoint set in it"
+
 
 mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
 
@@ -75,7 +76,7 @@ mi_gdb_test "405-break-insert subroutine
   "insert breakpoint subroutine"
 
 mi_gdb_test "406-data-evaluate-expression have_a_very_merry_interrupt()" \
-  "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
+  "406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
   "data evaluate expression"
 
 # We should have both a signal handler and a call dummy frame
@@ -99,7 +100,7 @@ mi_gdb_test "409-stack-list-frames 0 0" 
 # 
 
 mi_gdb_test "410-data-evaluate-expression bar()" \
-  "\\&\"The program being debugged was signaled while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"GDB remains in the frame where the signal was received.\\\\n\"\[\r\n\]+\\&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\n\"\[\r\n\]+\\&\"Evaluation of the expression containing the function \\(bar\\) will be abandoned.\\\\n\"\[\r\n\]+410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" \
+  "410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" \
   "call inferior function which raises exception"
 
 mi_gdb_test "411-stack-list-frames" "411\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
Index: src/gdb/testsuite/gdb.mi/mi2-syn-frame.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-syn-frame.exp	2008-03-23 19:56:34.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-syn-frame.exp	2008-03-23 19:56:36.000000000 +0000
@@ -50,7 +50,7 @@ mi_gdb_test "400-break-insert foo" \
 # Call foo() by hand, where we'll hit a breakpoint.
 #
 
-mi_gdb_test "401-data-evaluate-expression foo()" "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(foo\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.*\"" "call inferior's function with a breakpoint set in it"
+mi_gdb_test "401-data-evaluate-expression foo()" "401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(foo\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" "call inferior's function with a breakpoint set in it"
 
 mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
 
@@ -82,7 +82,7 @@ mi_gdb_test "405-break-insert subroutine
   "insert breakpoint subroutine"
 
 mi_gdb_test "406-data-evaluate-expression have_a_very_merry_interrupt()" \
-  "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
+  "406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
   "evaluate expression have_a_very_merry_interrupt"
 
 # We should have both a signal handler and a call dummy frame
@@ -111,7 +111,7 @@ mi_gdb_test "409-stack-list-frames 0 0" 
 # Call bar() by hand, which should get an exception while running.
 # 
 
-mi_gdb_test "410-data-evaluate-expression bar()" "\\&\"The program being debugged was signaled while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"GDB remains in the frame where the signal was received.\\\\n\"\[\r\n\]+\\&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\n\"\[\r\n\]+\\&\"Evaluation of the expression containing the function \\(bar\\) will be abandoned.\\\\n\"\[\r\n\]+410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" "call inferior function which raises exception"
+mi_gdb_test "410-data-evaluate-expression bar()" "410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" "call inferior function which raises exception"
 
 mi_gdb_test "411-stack-list-frames" "411\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-24 18:30 cleanup mi error message handling Pedro Alves
@ 2008-03-24 19:08 ` Pedro Alves
  2008-03-24 22:04 ` Nick Roberts
  2008-04-04 13:33 ` Vladimir Prus
  2 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2008-03-24 19:08 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1698 bytes --]

A Monday 24 March 2008 18:30:11, Pedro Alves wrote:
> I was looking through cleaning up mi/ global state, and noticed
> the global mi_error_message used to output ^error,msg=""
> messages.
>
> I then noticed that the errors output by the mi/ module are
> also duplicated to stderr.
>
> >./gdb -i=mi
>
> ~"GNU gdb 6.8.50.20080323-cvs\n"
> ~"Copyright (C) 2008 Free Software Foundation, Inc.\n"
> ~"License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>\n"
> ~"This is free software: you are free to change and redistribute it.\n"
> ~"There is NO WARRANTY, to the extent permitted by law.  Type \"show
> copying\"\n"
> ~"and \"show warranty\" for details.\n"
> ~"This GDB was configured as \"x86_64-unknown-linux-gnu\".\n"
> ~"Setting up the environment for debugging gdb.\n"
> &"/home/pedro/gdb/head/build/gdb/.gdbinit:5: Error in sourced command
> file:\n" &"No symbol table is loaded.  Use the \"file\" command.\n"
> (gdb)
> -exec-continue
> ^running
> (gdb)
> &"The program is not being run.\n"
> ^error,msg="The program is not being run."
> (gdb)
>
>
> Notice the duplication:
> &"The program is not being run.\n"
> ^error,msg="The program is not being run."
>
> I can't find a valid reason for that.
> In fact having them display in gdb's output window in a
> frontend doesn't help anyone, especially, since ^error is supposed
> to be the error output channel.  Furthermore, there are a couple
> of cases that weren't using the mechanism correctly, as they
> were already calling error.
>
> The attached patch fixes it, and fixes the testsuite to not
> expect this duplication.

I noticed post-submitting that I missed one cleanup.  Fixed as
attached.

-- 
Pedro Alves

[-- Attachment #2: mi_exceptions.diff --]
[-- Type: text/x-diff, Size: 46534 bytes --]

gdb/
2008-03-24  Pedro Alves  <pedro@codesourcery.com>

	* mi/mi-cmds.h (enum mi_cmd_result): Delete MI_CMD_ERROR.
	(mi_error_message): Delete declaration.
	* mi/mi-interp.c (mi_cmd_interpreter_exec): Call error instead of
	returning MI_CMD_ERROR.
	* mi/mi-main.c (mi_error_message): Delete.
	(mi_cmd_exec_interrupt):
	(mi_cmd_thread_select, mi_cmd_thread_list_ids)
	(mi_cmd_thread_info): Call error instead of returning
	MI_CMD_ERROR.
	(mi_cmd_data_list_register_values): Call error instead of
	returning MI_CMD_ERROR.  Adapt to new get_register interface.
	(get_register): Change return typo to void.  Call error instead of
	returning MI_CMD_ERROR.
	(mi_cmd_data_write_register_values): Call error instead of
	returning MI_CMD_ERROR.
	(mi_cmd_list_features): Return MI_CMD_DONE.
	(captured_mi_execute_command): Remove MI_CMD_ERROR handling.
	(mi_execute_command): Always print exceptions with -error.
	

gdb/testsuite/
2008-03-24  Pedro Alves  <pedro@codesourcery.com>

	* gdb.mi/mi-disassemble.exp, gdb.mi/mi-stack.exp,
	gdb.mi/mi-syn-frame.exp, gdb.mi/mi-var-block.exp,
	gdb.mi/mi-var-cmd.exp, gdb.mi/mi-var-display.exp,
	gdb.mi/mi2-disassemble.exp, gdb.mi/mi2-stack.exp,
	gdb.mi/mi2-syn-frame.exp, gdb.mi/mi2-var-block.exp,
	gdb.mi/mi2-var-cmd.exp, gdb.mi/mi2-var-display.exp: Update to not
	expect an mi error duplicated in stderr.

---
 gdb/mi/mi-cmds.h                         |    5 
 gdb/mi/mi-interp.c                       |   28 +--
 gdb/mi/mi-main.c                         |  251 +++++++++----------------------
 gdb/testsuite/gdb.mi/mi-disassemble.exp  |    8 
 gdb/testsuite/gdb.mi/mi-stack.exp        |    8 
 gdb/testsuite/gdb.mi/mi-syn-frame.exp    |    7 
 gdb/testsuite/gdb.mi/mi-var-block.exp    |    4 
 gdb/testsuite/gdb.mi/mi-var-cmd.exp      |   10 -
 gdb/testsuite/gdb.mi/mi-var-display.exp  |    2 
 gdb/testsuite/gdb.mi/mi2-disassemble.exp |    8 
 gdb/testsuite/gdb.mi/mi2-stack.exp       |    8 
 gdb/testsuite/gdb.mi/mi2-syn-frame.exp   |    6 
 gdb/testsuite/gdb.mi/mi2-var-block.exp   |    4 
 gdb/testsuite/gdb.mi/mi2-var-cmd.exp     |   10 -
 gdb/testsuite/gdb.mi/mi2-var-display.exp |    2 
 15 files changed, 130 insertions(+), 231 deletions(-)

Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/mi/mi-main.c	2008-03-24 17:53:20.000000000 +0000
@@ -96,7 +96,6 @@ static int do_timings = 0;
 /* The token of the last asynchronous command.  */
 static char *last_async_command;
 static char *previous_async_command;
-char *mi_error_message;
 
 extern void _initialize_mi_main (void);
 static enum mi_cmd_result mi_cmd_execute (struct mi_parse *parse);
@@ -109,7 +108,7 @@ static void mi_exec_async_cli_cmd_contin
 
 static int register_changed_p (int regnum, struct regcache *,
 			       struct regcache *);
-static int get_register (int regnum, int format);
+static void get_register (int regnum, int format);
 
 /* Command implementations.  FIXME: Is this libgdb?  No.  This is the MI
    layer that calls libgdb.  Any operation used in the below should be
@@ -219,10 +218,8 @@ enum mi_cmd_result
 mi_cmd_exec_interrupt (char *args, int from_tty)
 {
   if (!target_executing)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_exec_interrupt: Inferior not executing.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_exec_interrupt: Inferior not executing.");
+
   interrupt_target_command (args, from_tty);
   if (last_async_command)
     fputs_unfiltered (last_async_command, raw_stdout);
@@ -242,38 +239,40 @@ enum mi_cmd_result
 mi_cmd_thread_select (char *command, char **argv, int argc)
 {
   enum gdb_rc rc;
+  char *mi_error_message;
 
   if (argc != 1)
+    error ("mi_cmd_thread_select: USAGE: threadnum.");
+
+  rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
+
+  if (rc == GDB_RC_FAIL)
     {
-      mi_error_message = xstrprintf ("mi_cmd_thread_select: USAGE: threadnum.");
-      return MI_CMD_ERROR;
+      make_cleanup (xfree, mi_error_message);
+      error ("%s", mi_error_message);
     }
-  else
-    rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
 
-  if (rc == GDB_RC_FAIL)
-    return MI_CMD_ERROR;
-  else
-    return MI_CMD_DONE;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
 mi_cmd_thread_list_ids (char *command, char **argv, int argc)
 {
   enum gdb_rc rc;
+  char *mi_error_message;
 
   if (argc != 0)
+    error ("mi_cmd_thread_list_ids: No arguments required.");
+
+  rc = gdb_list_thread_ids (uiout, &mi_error_message);
+
+  if (rc == GDB_RC_FAIL)
     {
-      mi_error_message = xstrprintf ("mi_cmd_thread_list_ids: No arguments required.");
-      return MI_CMD_ERROR;
+      make_cleanup (xfree, mi_error_message);
+      error ("%s", mi_error_message);
     }
-  else
-    rc = gdb_list_thread_ids (uiout, &mi_error_message);
 
-  if (rc == GDB_RC_FAIL)
-    return MI_CMD_ERROR;
-  else
-    return MI_CMD_DONE;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
@@ -282,10 +281,7 @@ mi_cmd_thread_info (char *command, char 
   int thread = -1;
   
   if (argc != 0 && argc != 1)
-    {
-      mi_error_message = xstrprintf ("Invalid MI command");
-      return MI_CMD_ERROR;
-    }
+    error ("Invalid MI command");
 
   if (argc == 1)
     thread = atoi (argv[0]);
@@ -333,11 +329,8 @@ mi_cmd_data_list_register_names (char *c
     {
       regnum = atoi (argv[i]);
       if (regnum < 0 || regnum >= numregs)
-	{
-	  do_cleanups (cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
+
       if (gdbarch_register_name (current_gdbarch, regnum) == NULL
 	  || *(gdbarch_register_name (current_gdbarch, regnum)) == '\0')
 	ui_out_field_string (uiout, NULL, "");
@@ -388,11 +381,7 @@ mi_cmd_data_list_changed_registers (char
 	    continue;
 	  changed = register_changed_p (regnum, prev_regs, this_regs);
 	  if (changed < 0)
-	    {
-	      do_cleanups (cleanup);
-	      mi_error_message = xstrprintf ("mi_cmd_data_list_changed_registers: Unable to read register contents.");
-	      return MI_CMD_ERROR;
-	    }
+	    error ("mi_cmd_data_list_changed_registers: Unable to read register contents.");
 	  else if (changed)
 	    ui_out_field_int (uiout, NULL, regnum);
 	}
@@ -410,20 +399,12 @@ mi_cmd_data_list_changed_registers (char
 	{
 	  changed = register_changed_p (regnum, prev_regs, this_regs);
 	  if (changed < 0)
-	    {
-	      do_cleanups (cleanup);
-	      mi_error_message = xstrprintf ("mi_cmd_data_list_register_change: Unable to read register contents.");
-	      return MI_CMD_ERROR;
-	    }
+	    error ("mi_cmd_data_list_register_change: Unable to read register contents.");
 	  else if (changed)
 	    ui_out_field_int (uiout, NULL, regnum);
 	}
       else
-	{
-	  do_cleanups (cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   do_cleanups (cleanup);
   return MI_CMD_DONE;
@@ -465,7 +446,7 @@ register_changed_p (int regnum, struct r
 enum mi_cmd_result
 mi_cmd_data_list_register_values (char *command, char **argv, int argc)
 {
-  int regnum, numregs, format, result;
+  int regnum, numregs, format;
   int i;
   struct cleanup *list_cleanup, *tuple_cleanup;
 
@@ -479,10 +460,7 @@ mi_cmd_data_list_register_values (char *
 	    + gdbarch_num_pseudo_regs (current_gdbarch);
 
   if (argc == 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: Usage: -data-list-register-values <format> [<regnum1>...<regnumN>]");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_list_register_values: Usage: -data-list-register-values <format> [<regnum1>...<regnumN>]");
 
   format = (int) argv[0][0];
 
@@ -499,12 +477,7 @@ mi_cmd_data_list_register_values (char *
 	    continue;
 	  tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_int (uiout, "number", regnum);
-	  result = get_register (regnum, format);
-	  if (result == -1)
-	    {
-	      do_cleanups (list_cleanup);
-	      return MI_CMD_ERROR;
-	    }
+	  get_register (regnum, format);
 	  do_cleanups (tuple_cleanup);
 	}
     }
@@ -521,27 +494,18 @@ mi_cmd_data_list_register_values (char *
 	{
 	  tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_int (uiout, "number", regnum);
-	  result = get_register (regnum, format);
-	  if (result == -1)
-	    {
-	      do_cleanups (list_cleanup);
-	      return MI_CMD_ERROR;
-	    }
+	  get_register (regnum, format);
 	  do_cleanups (tuple_cleanup);
 	}
       else
-	{
-	  do_cleanups (list_cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   do_cleanups (list_cleanup);
   return MI_CMD_DONE;
 }
 
 /* Output one register's contents in the desired format.  */
-static int
+static void
 get_register (int regnum, int format)
 {
   gdb_byte buffer[MAX_REGISTER_SIZE];
@@ -560,10 +524,7 @@ get_register (int regnum, int format)
 		  &realnum, buffer);
 
   if (optim)
-    {
-      mi_error_message = xstrprintf ("Optimized out");
-      return -1;
-    }
+    error ("Optimized out");
 
   if (format == 'r')
     {
@@ -589,7 +550,6 @@ get_register (int regnum, int format)
       ui_out_field_stream (uiout, "value", stb);
       ui_out_stream_delete (stb);
     }
-  return 1;
 }
 
 /* Write given values into registers. The registers and values are
@@ -611,30 +571,18 @@ mi_cmd_data_write_register_values (char 
 	    + gdbarch_num_pseudo_regs (current_gdbarch);
 
   if (argc == 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: Usage: -data-write-register-values <format> [<regnum1> <value1>...<regnumN> <valueN>]");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: Usage: -data-write-register-values <format> [<regnum1> <value1>...<regnumN> <valueN>]");
 
   format = (int) argv[0][0];
 
   if (!target_has_registers)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: No registers.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: No registers.");
 
   if (!(argc - 1))
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: No regs and values specified.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: No regs and values specified.");
 
   if ((argc - 1) % 2)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: Regs and vals are not in pairs.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: Regs and vals are not in pairs.");
 
   for (i = 1; i < argc; i = i + 2)
     {
@@ -653,10 +601,7 @@ mi_cmd_data_write_register_values (char 
 	  regcache_cooked_write_signed (get_current_regcache (), regnum, value);
 	}
       else
-	{
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   return MI_CMD_DONE;
 }
@@ -676,9 +621,8 @@ mi_cmd_data_evaluate_expression (char *c
 
   if (argc != 1)
     {
-      mi_error_message = xstrprintf ("mi_cmd_data_evaluate_expression: Usage: -data-evaluate-expression expression");
       ui_out_stream_delete (stb);
-      return MI_CMD_ERROR;
+      error ("mi_cmd_data_evaluate_expression: Usage: -data-evaluate-expression expression");
     }
 
   expr = parse_expression (argv[0]);
@@ -808,10 +752,7 @@ mi_cmd_data_read_memory (char *command, 
   argc -= optind;
 
   if (argc < 5 || argc > 6)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: Usage: ADDR WORD-FORMAT WORD-SIZE NR-ROWS NR-COLS [ASCHAR].");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: Usage: ADDR WORD-FORMAT WORD-SIZE NR-ROWS NR-COLS [ASCHAR].");
 
   /* Extract all the arguments. */
 
@@ -847,17 +788,13 @@ mi_cmd_data_read_memory (char *command, 
   /* The number of rows.  */
   nr_rows = atol (argv[3]);
   if (nr_rows <= 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: invalid number of rows.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: invalid number of rows.");
+
   /* Number of bytes per row.  */
   nr_cols = atol (argv[4]);
   if (nr_cols <= 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: invalid number of columns.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: invalid number of columns.");
+
   /* The un-printable character when printing ascii.  */
   if (argc == 6)
     aschar = *argv[5];
@@ -872,11 +809,7 @@ mi_cmd_data_read_memory (char *command, 
   nr_bytes = target_read (&current_target, TARGET_OBJECT_MEMORY, NULL,
 			  mbuf, addr, total_bytes);
   if (nr_bytes <= 0)
-    {
-      do_cleanups (cleanups);
-      mi_error_message = xstrdup ("Unable to read memory.");
-      return MI_CMD_ERROR;
-    }
+    error ("Unable to read memory.");
 
   /* Output the header information.  */
   ui_out_field_core_addr (uiout, "addr", addr);
@@ -1008,10 +941,7 @@ mi_cmd_data_write_memory (char *command,
   argc -= optind;
 
   if (argc != 4)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_memory: Usage: [-o COLUMN_OFFSET] ADDR FORMAT WORD-SIZE VALUE.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_memory: Usage: [-o COLUMN_OFFSET] ADDR FORMAT WORD-SIZE VALUE.");
 
   /* Extract all the arguments.  */
   /* Start address of the memory dump.  */
@@ -1060,7 +990,7 @@ mi_cmd_enable_timings (char *command, ch
 
  usage_error:
   error ("mi_cmd_enable_timings: Usage: %s {yes|no}", command);
-  return MI_CMD_ERROR;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
@@ -1081,7 +1011,7 @@ mi_cmd_list_features (char *command, cha
     }
 
   error ("-list-features should be passed no arguments");
-  return MI_CMD_ERROR;
+  return MI_CMD_DONE;
 }
  
 /* Execute a command within a safe environment.
@@ -1121,7 +1051,7 @@ captured_mi_execute_command (struct ui_o
       args->rc = mi_cmd_execute (context);
 
       if (do_timings)
-          timestamp (&cmd_finished);
+	timestamp (&cmd_finished);
 
       if (!target_can_async_p () || !target_executing)
 	{
@@ -1143,19 +1073,6 @@ captured_mi_execute_command (struct ui_o
 		  print_diff (context->cmd_start, &cmd_finished);
 	      fputs_unfiltered ("\n", raw_stdout);
 	    }
-	  else if (args->rc == MI_CMD_ERROR)
-	    {
-	      if (mi_error_message)
-		{
-		  fputs_unfiltered (context->token, raw_stdout);
-		  fputs_unfiltered ("^error,msg=\"", raw_stdout);
-		  fputstr_unfiltered (mi_error_message, '"', raw_stdout);
-		  xfree (mi_error_message);
-		  mi_error_message = NULL;
-		  fputs_unfiltered ("\"\n", raw_stdout);
-		}
-	      mi_out_rewind (uiout);
-	    }
 	  else
 	    mi_out_rewind (uiout);
 	}
@@ -1196,19 +1113,6 @@ captured_mi_execute_command (struct ui_o
 		fputs_unfiltered ("\n", raw_stdout);
 		args->action = EXECUTE_COMMAND_DISPLAY_PROMPT;
 	      }
-	    else if (args->rc == MI_CMD_ERROR)
-	      {
-		if (mi_error_message)
-		  {
-		    fputs_unfiltered (context->token, raw_stdout);
-		    fputs_unfiltered ("^error,msg=\"", raw_stdout);
-		    fputstr_unfiltered (mi_error_message, '"', raw_stdout);
-		    xfree (mi_error_message);
-		    mi_error_message = NULL;
-		    fputs_unfiltered ("\"\n", raw_stdout);
-		  }
-		mi_out_rewind (uiout);
-	      }
 	    else
 	      mi_out_rewind (uiout);
 	  }
@@ -1246,20 +1150,9 @@ mi_execute_command (char *cmd, int from_
 	  timestamp (command->cmd_start);
 	}
 
-      /* FIXME: cagney/1999-11-04: Can this use of catch_exceptions either
-         be pushed even further down or even eliminated?  */
       args.command = command;
       result = catch_exception (uiout, captured_mi_execute_command, &args,
 				RETURN_MASK_ALL);
-      exception_print (gdb_stderr, result);
-
-      if (args.action == EXECUTE_COMMAND_SUPRESS_PROMPT)
-	{
-	  /* The command is executing synchronously.  Bail out early
-	     suppressing the finished prompt.  */
-	  mi_parse_free (command);
-	  return;
-	}
       if (result.reason < 0)
 	{
 	  /* The command execution failed and error() was called
@@ -1269,11 +1162,17 @@ mi_execute_command (char *cmd, int from_
 	  if (result.message == NULL)
 	    fputs_unfiltered ("unknown error", raw_stdout);
 	  else
-	      fputstr_unfiltered (result.message, '"', raw_stdout);
+	    fputstr_unfiltered (result.message, '"', raw_stdout);
 	  fputs_unfiltered ("\"\n", raw_stdout);
 	  mi_out_rewind (uiout);
 	}
+
       mi_parse_free (command);
+
+      if (args.action == EXECUTE_COMMAND_SUPRESS_PROMPT)
+	/* The command is executing synchronously.  Bail out early
+	   suppressing the finished prompt.  */
+	return;
     }
 
   fputs_unfiltered ("(gdb) \n", raw_stdout);
@@ -1311,13 +1210,15 @@ mi_cmd_execute (struct mi_parse *parse)
 	    previous_async_command = xstrdup (last_async_command);
 	  if (strcmp (parse->command, "exec-interrupt"))
 	    {
-	      fputs_unfiltered (parse->token, raw_stdout);
-	      fputs_unfiltered ("^error,msg=\"", raw_stdout);
-	      fputs_unfiltered ("Cannot execute command ", raw_stdout);
-	      fputstr_unfiltered (parse->command, '"', raw_stdout);
-	      fputs_unfiltered (" while target running", raw_stdout);
-	      fputs_unfiltered ("\"\n", raw_stdout);
-	      return MI_CMD_ERROR;
+	      struct ui_file *stb;
+	      stb = mem_fileopen ();
+
+	      fputs_unfiltered ("Cannot execute command ", stb);
+	      fputstr_unfiltered (parse->command, '"', stb);
+	      fputs_unfiltered (" while target running", stb);
+
+	      make_cleanup_ui_file_delete (stb);
+	      error_stream (stb);
 	    }
 	}
       last_async_command = xstrdup (parse->token);
@@ -1339,13 +1240,19 @@ mi_cmd_execute (struct mi_parse *parse)
   else
     {
       /* FIXME: DELETE THIS.  */
-      fputs_unfiltered (parse->token, raw_stdout);
-      fputs_unfiltered ("^error,msg=\"", raw_stdout);
-      fputs_unfiltered ("Undefined mi command: ", raw_stdout);
-      fputstr_unfiltered (parse->command, '"', raw_stdout);
-      fputs_unfiltered (" (missing implementation)", raw_stdout);
-      fputs_unfiltered ("\"\n", raw_stdout);
-      return MI_CMD_ERROR;
+      struct ui_file *stb;
+
+      stb = mem_fileopen ();
+
+      fputs_unfiltered ("Undefined mi command: ", stb);
+      fputstr_unfiltered (parse->command, '"', stb);
+      fputs_unfiltered (" (missing implementation)", stb);
+
+      make_cleanup_ui_file_delete (stb);
+      error_stream (stb);
+
+      /* unreacheable */
+      return MI_CMD_DONE;
     }
 }
 
Index: src/gdb/mi/mi-cmds.h
===================================================================
--- src.orig/gdb/mi/mi-cmds.h	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/mi/mi-cmds.h	2008-03-24 17:53:20.000000000 +0000
@@ -33,10 +33,6 @@ enum mi_cmd_result
     /* The command is still running in the forground.  Main loop should
        display the completion prompt. */
     MI_CMD_FORGROUND,
-    /* An error condition was detected and an error message was
-       asprintf'd into the mi_error_message buffer.  The main loop will
-       display the error message and the completion prompt. */
-    MI_CMD_ERROR,
     /* The MI command has already displayed its completion message.
        Main loop will not display a completion message but will display
        the completion prompt. */
@@ -156,7 +152,6 @@ extern int mi_debug_p;
 /* Raw console output - FIXME: should this be a parameter? */
 extern struct ui_file *raw_stdout;
 
-extern char *mi_error_message;
 extern void mi_execute_command (char *cmd, int from_tty);
 
 #endif
Index: src/gdb/mi/mi-interp.c
===================================================================
--- src.orig/gdb/mi/mi-interp.c	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/mi/mi-interp.c	2008-03-24 18:38:56.000000000 +0000
@@ -190,29 +190,21 @@ enum mi_cmd_result
 mi_cmd_interpreter_exec (char *command, char **argv, int argc)
 {
   struct interp *interp_to_use;
-  enum mi_cmd_result result = MI_CMD_DONE;
   int i;
   struct interp_procs *procs;
+  char *mi_error_message = NULL;
+  struct cleanup *old_chain;
 
   if (argc < 2)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command");
 
   interp_to_use = interp_lookup (argv[0]);
   if (interp_to_use == NULL)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: could not find interpreter \"%s\"", argv[0]);
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: could not find interpreter \"%s\"", argv[0]);
 
   if (!interp_exec_p (interp_to_use))
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: interpreter \"%s\" does not support command execution",
-				     argv[0]);
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: interpreter \"%s\" does not support command execution",
+	      argv[0]);
 
   /* Insert the MI out hooks, making sure to also call the interpreter's hooks
      if it has any. */
@@ -222,13 +214,14 @@ mi_cmd_interpreter_exec (char *command, 
 
   /* Now run the code... */
 
+  old_chain = make_cleanup (null_cleanup, 0);
   for (i = 1; i < argc; i++)
     {
       struct gdb_exception e = interp_exec (interp_to_use, argv[i]);
       if (e.reason < 0)
 	{
 	  mi_error_message = xstrdup (e.message);
-	  result = MI_CMD_ERROR;
+	  make_cleanup (xfree, mi_error_message);
 	  break;
 	}
     }
@@ -246,7 +239,10 @@ mi_cmd_interpreter_exec (char *command, 
       add_continuation (mi_interpreter_exec_continuation, NULL);
     }
 
-  return result;
+  if (mi_error_message != NULL)
+    error ("%s", mi_error_message);
+  do_cleanups (old_chain);
+  return MI_CMD_DONE;
 }
 
 /*
Index: src/gdb/testsuite/gdb.mi/mi-disassemble.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-disassemble.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-disassemble.exp	2008-03-24 17:53:20.000000000 +0000
@@ -160,19 +160,19 @@ proc test_disassembly_bogus_args {} {
     # -data-disassembly -f basics.c -l 32 -- 9
 
     mi_gdb_test "123-data-disassemble -f foo -l abc -n 0 -- 0" \
-             "&.*123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
+             "123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
              "data-disassemble bogus filename"
 
     mi_gdb_test "321-data-disassemble -s foo -e bar -- 0" \
-             "&.*321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
+             "321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
              "data-disassemble bogus address"
 
     mi_gdb_test "456-data-disassemble -s \$pc -f basics.c -- 0" \
-             "&.*456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
+             "456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
              "data-disassemble mix different args"
 
     mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
-             "&.*789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
+             "789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
              "data-disassemble wrong mode arg"
 
 }
Index: src/gdb/testsuite/gdb.mi/mi-stack.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-stack.exp	2008-03-24 17:28:16.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-stack.exp	2008-03-24 17:53:20.000000000 +0000
@@ -69,7 +69,7 @@ proc test_stack_frame_listing {} {
                 "stack frame listing 1 3"
 
     mi_gdb_test "234-stack-list-frames 1" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack frame listing wrong"
 
     mi_gdb_test "235-stack-info-frame" \
@@ -120,7 +120,7 @@ proc test_stack_args_listing {} {
                 "stack args listing 1 1 3"
 
     mi_gdb_test "234-stack-list-arguments" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack args listing wrong"
 
     mi_gdb_test "235-stack-list-arguments 1 1 300" \
@@ -151,7 +151,7 @@ proc test_stack_info_depth {} {
                 "stack info-depth 99"
 
     mi_gdb_test "231-stack-info-depth 99 99" \
-	    "&.*231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
+	    "231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
                 "stack info-depth wrong usage"
 }
 
@@ -190,7 +190,7 @@ gdb_expect {
   "stack locals listing, simple types: names and values, complex type: names and types"
 
     mi_gdb_test "234-stack-list-locals" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
 	    "stack locals listing wrong"
 
     mi_gdb_test "232-stack-select-frame 1" \
Index: src/gdb/testsuite/gdb.mi/mi-var-block.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-block.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-var-block.exp	2008-03-24 17:53:20.000000000 +0000
@@ -49,7 +49,7 @@ mi_gdb_test "-var-create cb * cb" \
 	"create local variable cb"
 
 mi_gdb_test "-var-create foo * foo" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create local variable foo"
 
 # step to "foo = 123;"
@@ -60,7 +60,7 @@ mi_step_to "do_block_tests" "" "var-cmd.
 
 # Be paranoid and assume 3.2 created foo
 mi_gdb_test "-var-delete foo" \
-	"&\"Variable object not found\\\\n\".*\\^error,msg=\"Variable object not found\"" \
+	"\\^error,msg=\"Variable object not found\"" \
 	"delete var foo"
 
 
Index: src/gdb/testsuite/gdb.mi/mi-var-cmd.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-cmd.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-var-cmd.exp	2008-03-24 17:53:20.000000000 +0000
@@ -58,14 +58,14 @@ mi_gdb_test "111-var-create global_simpl
 # Desc: Create non-existent variable
 
 mi_gdb_test "112-var-create bogus_unknown_variable * bogus_unknown_variable" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create non-existent variable"
 
 # Test: c_variable-1.3
 # Desc: Create out of scope variable
 
 mi_gdb_test "113-var-create argc * argc" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create out of scope variable"
 
 mi_runto do_locals_tests
@@ -154,7 +154,7 @@ mi_gdb_test "-var-create lsimple.integer
 #    Type names (like int, long, etc..) are all proper expressions to gdb.
 #    make sure variable code does not allow users to create variables, though.
 mi_gdb_test "-var-create int * int" \
-	"&\"Attempt to use a type name as an expression.\\\\n\".*&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"&\"Attempt to use a type name as an expression.\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create int"
 
 
@@ -275,7 +275,7 @@ mi_gdb_test "-var-update *" \
 #
 ###
 mi_gdb_test "-var-assign global_simple 0" \
-	"&\"mi_cmd_var_assign: Variable object is not editable\\\\n\".*\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
+	"\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
 	"assign to global_simple"
 
 mi_gdb_test "-var-assign linteger 3333" \
@@ -449,7 +449,7 @@ mi_gdb_test "-var-create l * l" \
 # Test: c_variable-2.11
 # Desc: create do_locals_tests local in subroutine1
 mi_gdb_test "-var-create linteger * linteger" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create linteger"
 
 mi_step_to "subroutine1" "\{name=\"i\",value=\".*\"\},\{name=\"l\",value=\".*\"\}" \
Index: src/gdb/testsuite/gdb.mi/mi-var-display.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-display.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-var-display.exp	2008-03-24 17:53:20.000000000 +0000
@@ -544,7 +544,7 @@ clear_xfail "*-*-*"
 # Test: c_variable-7.70
 # Desc: create anone
 mi_gdb_test "-var-create anone * anone" \
-	"&\"Duplicate variable object name\\\\n\".*\\^error,msg=\"Duplicate variable object name\"" \
+	"\\^error,msg=\"Duplicate variable object name\"" \
 	"create duplicate local variable anone"
 
 
Index: src/gdb/testsuite/gdb.mi/mi2-disassemble.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-disassemble.exp	2008-03-24 17:28:16.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-disassemble.exp	2008-03-24 17:53:20.000000000 +0000
@@ -160,19 +160,19 @@ proc test_disassembly_bogus_args {} {
     # -data-disassembly -f basics.c -l 32 -- 9
 
     mi_gdb_test "123-data-disassemble -f foo -l abc -n 0 -- 0" \
-             "&.*123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
+             "123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
              "data-disassemble bogus filename"
 
     mi_gdb_test "321-data-disassemble -s foo -e bar -- 0" \
-             "&.*321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
+             "321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
              "data-disassemble bogus address"
 
     mi_gdb_test "456-data-disassemble -s \$pc -f basics.c -- 0" \
-             "&.*456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
+             "456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
              "data-disassemble mix different args"
 
     mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
-             "&.*789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
+             "789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
              "data-disassemble wrong mode arg"
 
 }
Index: src/gdb/testsuite/gdb.mi/mi2-stack.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-stack.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-stack.exp	2008-03-24 17:53:20.000000000 +0000
@@ -69,7 +69,7 @@ proc test_stack_frame_listing {} {
                 "stack frame listing 1 3"
 
     mi_gdb_test "234-stack-list-frames 1" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack frame listing wrong"
 
     mi_gdb_test "235-stack-info-frame" \
@@ -120,7 +120,7 @@ proc test_stack_args_listing {} {
                 "stack args listing 1 1 3"
 
     mi_gdb_test "234-stack-list-arguments" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack args listing wrong"
 
     mi_gdb_test "235-stack-list-arguments 1 1 300" \
@@ -151,7 +151,7 @@ proc test_stack_info_depth {} {
                 "stack info-depth 99"
 
     mi_gdb_test "231-stack-info-depth 99 99" \
-	    "&.*231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
+	    "231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
                 "stack info-depth wrong usage"
 }
 
@@ -190,7 +190,7 @@ gdb_expect {
   "stack locals listing, simple types: names and values, complex type: names and types"
 
     mi_gdb_test "234-stack-list-locals" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
 	    "stack locals listing wrong"
 
     mi_gdb_test "232-stack-select-frame 1" \
Index: src/gdb/testsuite/gdb.mi/mi2-var-block.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-block.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-var-block.exp	2008-03-24 17:53:20.000000000 +0000
@@ -49,7 +49,7 @@ mi_gdb_test "-var-create cb * cb" \
 	"create local variable cb"
 
 mi_gdb_test "-var-create foo * foo" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create local variable foo"
 
 # step to "foo = 123;"
@@ -60,7 +60,7 @@ mi_step_to "do_block_tests" "" "var-cmd.
 
 # Be paranoid and assume 3.2 created foo
 mi_gdb_test "-var-delete foo" \
-	"&\"Variable object not found\\\\n\".*\\^error,msg=\"Variable object not found\"" \
+	"\\^error,msg=\"Variable object not found\"" \
 	"delete var foo"
 
 
Index: src/gdb/testsuite/gdb.mi/mi2-var-cmd.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-cmd.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-var-cmd.exp	2008-03-24 17:53:20.000000000 +0000
@@ -58,14 +58,14 @@ mi_gdb_test "111-var-create global_simpl
 # Desc: Create non-existent variable
 
 mi_gdb_test "112-var-create bogus_unknown_variable * bogus_unknown_variable" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create non-existent variable"
 
 # Test: c_variable-1.3
 # Desc: Create out of scope variable
 
 mi_gdb_test "113-var-create argc * argc" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create out of scope variable"
 
 mi_runto do_locals_tests
@@ -154,7 +154,7 @@ mi_gdb_test "-var-create lsimple.integer
 #    Type names (like int, long, etc..) are all proper expressions to gdb.
 #    make sure variable code does not allow users to create variables, though.
 mi_gdb_test "-var-create int * int" \
-	"&\"Attempt to use a type name as an expression.\\\\n\".*&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"&\"Attempt to use a type name as an expression.\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create int"
 
 
@@ -275,7 +275,7 @@ mi_gdb_test "-var-update *" \
 #
 ###
 mi_gdb_test "-var-assign global_simple 0" \
-	"&\"mi_cmd_var_assign: Variable object is not editable\\\\n\".*\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
+	"\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
 	"assign to global_simple"
 
 mi_gdb_test "-var-assign linteger 3333" \
@@ -412,7 +412,7 @@ mi_gdb_test "-var-create l * l" \
 # Test: c_variable-2.11
 # Desc: create do_locals_tests local in subroutine1
 mi_gdb_test "-var-create linteger * linteger" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create linteger"
 
 mi_step_to "subroutine1" "\{name=\"i\",value=\".*\"\},\{name=\"l\",value=\".*\"\}" \
Index: src/gdb/testsuite/gdb.mi/mi2-var-display.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-display.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-var-display.exp	2008-03-24 17:53:20.000000000 +0000
@@ -544,7 +544,7 @@ clear_xfail "*-*-*"
 # Test: c_variable-7.70
 # Desc: create anone
 mi_gdb_test "-var-create anone * anone" \
-	"&\"Duplicate variable object name\\\\n\".*\\^error,msg=\"Duplicate variable object name\"" \
+	"\\^error,msg=\"Duplicate variable object name\"" \
 	"create duplicate local variable anone"
 
 
Index: src/gdb/testsuite/gdb.mi/mi-syn-frame.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-syn-frame.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi-syn-frame.exp	2008-03-24 17:53:20.000000000 +0000
@@ -48,7 +48,8 @@ mi_gdb_test "400-break-insert foo" \
 # Call foo() by hand, where we'll hit a breakpoint.
 #
 
-mi_gdb_test "401-data-evaluate-expression foo()" "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(foo\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.*\"" "call inferior's function with a breakpoint set in it"
+mi_gdb_test "401-data-evaluate-expression foo()" "401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(foo\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" "call inferior's function with a breakpoint set in it"
+
 
 mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
 
@@ -75,7 +76,7 @@ mi_gdb_test "405-break-insert subroutine
   "insert breakpoint subroutine"
 
 mi_gdb_test "406-data-evaluate-expression have_a_very_merry_interrupt()" \
-  "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
+  "406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
   "data evaluate expression"
 
 # We should have both a signal handler and a call dummy frame
@@ -99,7 +100,7 @@ mi_gdb_test "409-stack-list-frames 0 0" 
 # 
 
 mi_gdb_test "410-data-evaluate-expression bar()" \
-  "\\&\"The program being debugged was signaled while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"GDB remains in the frame where the signal was received.\\\\n\"\[\r\n\]+\\&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\n\"\[\r\n\]+\\&\"Evaluation of the expression containing the function \\(bar\\) will be abandoned.\\\\n\"\[\r\n\]+410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" \
+  "410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" \
   "call inferior function which raises exception"
 
 mi_gdb_test "411-stack-list-frames" "411\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
Index: src/gdb/testsuite/gdb.mi/mi2-syn-frame.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-syn-frame.exp	2008-03-24 17:28:15.000000000 +0000
+++ src/gdb/testsuite/gdb.mi/mi2-syn-frame.exp	2008-03-24 17:53:20.000000000 +0000
@@ -50,7 +50,7 @@ mi_gdb_test "400-break-insert foo" \
 # Call foo() by hand, where we'll hit a breakpoint.
 #
 
-mi_gdb_test "401-data-evaluate-expression foo()" "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(foo\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.*\"" "call inferior's function with a breakpoint set in it"
+mi_gdb_test "401-data-evaluate-expression foo()" "401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(foo\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" "call inferior's function with a breakpoint set in it"
 
 mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
 
@@ -82,7 +82,7 @@ mi_gdb_test "405-break-insert subroutine
   "insert breakpoint subroutine"
 
 mi_gdb_test "406-data-evaluate-expression have_a_very_merry_interrupt()" \
-  "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
+  "406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
   "evaluate expression have_a_very_merry_interrupt"
 
 # We should have both a signal handler and a call dummy frame
@@ -111,7 +111,7 @@ mi_gdb_test "409-stack-list-frames 0 0" 
 # Call bar() by hand, which should get an exception while running.
 # 
 
-mi_gdb_test "410-data-evaluate-expression bar()" "\\&\"The program being debugged was signaled while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"GDB remains in the frame where the signal was received.\\\\n\"\[\r\n\]+\\&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\n\"\[\r\n\]+\\&\"Evaluation of the expression containing the function \\(bar\\) will be abandoned.\\\\n\"\[\r\n\]+410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" "call inferior function which raises exception"
+mi_gdb_test "410-data-evaluate-expression bar()" "410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" "call inferior function which raises exception"
 
 mi_gdb_test "411-stack-list-frames" "411\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-24 18:30 cleanup mi error message handling Pedro Alves
  2008-03-24 19:08 ` Pedro Alves
@ 2008-03-24 22:04 ` Nick Roberts
  2008-03-24 22:39   ` Daniel Jacobowitz
  2008-03-25  3:52   ` Pedro Alves
  2008-04-04 13:33 ` Vladimir Prus
  2 siblings, 2 replies; 26+ messages in thread
From: Nick Roberts @ 2008-03-24 22:04 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

 > Notice the duplication:
 > &"The program is not being run.\n"
 > ^error,msg="The program is not being run."
 > 
 > I can't find a valid reason for that.
 > In fact having them display in gdb's output window in a
 > frontend doesn't help anyone, especially, since ^error is supposed
 > to be the error output channel.  Furthermore, there are a couple
 > of cases that weren't using the mechanism correctly, as they
 > were already calling error.
 > 
 > The attached patch fixes it, and fixes the testsuite to not
 > expect this duplication.
 > 
 > -- 
 > Pedro Alves
 > gdb/
 > 2008-03-24  Pedro Alves  <pedro@codesourcery.com>
 > 
 > 	* mi/mi-cmds.h (enum mi_cmd_result): Delete MI_CMD_ERROR.
 > 	(mi_error_message): Delete declaration.
 > 	* mi/mi-interp.c (mi_cmd_interpreter_exec): Call error instead of
 > 	returning MI_CMD_ERROR.
 > 	* mi/mi-main.c (mi_error_message): Delete.
 > 	(mi_cmd_exec_interrupt):
 > 	(mi_cmd_thread_select, mi_cmd_thread_list_ids)
 > 	(mi_cmd_thread_info): Call error instead of returning
 > 	MI_CMD_ERROR.
 > 	(mi_cmd_data_list_register_values): Call error instead of
 > 	returning MI_CMD_ERROR.  Adapt to new get_register interface.
 > 	(get_register): Change return typo to void.  Call error instead of
 > 	returning MI_CMD_ERROR.
 > 	(mi_cmd_data_write_register_values): Call error instead of
 > 	returning MI_CMD_ERROR.
 > 	(mi_cmd_list_features): Return MI_CMD_DONE.
 > 	(captured_mi_execute_command): Remove MI_CMD_ERROR handling.
 > 	(mi_execute_command): Always print exceptions with -error.

The error message isn't duplicated when MI_CMD_ERROR is used, but the bulk
of this patch is about removing it.  Duplication comes from the single
line:

     exception_print (gdb_stderr, result);

in mi_execute_command.

I think sometimes errors are just needed when debugging the frontend, e.g.,

  -thread-select
  ^error,msg="mi_cmd_thread_select: USAGE: threadnum."
  (gdb) 

where one error message is sufficient.  Here I think Gdb is trying to mimic
perror by giving the name of the C procedure, but I think it's not desirable as
this gets printed due to a frontend error, not a Gdb error.  So I would like to
see:

  -thread-select
  ^error,msg="USAGE: threadnum."
  (gdb) 

At other times duplicated error messages are desirable, e.g.,

  -exec-continue 
  ^running
  (gdb) 
  &"The program is not being run.\n"
  ^error,msg="The program is not being run."

because the first goes to the console for the user to see, the second to
the frontend to be handled as appropriate.

For this reason I would not like to see this patch applied.

I have previously suggested changes to MI's error reporting.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-24 22:04 ` Nick Roberts
@ 2008-03-24 22:39   ` Daniel Jacobowitz
  2008-03-24 23:37     ` Nick Roberts
  2008-03-25  3:52   ` Pedro Alves
  1 sibling, 1 reply; 26+ messages in thread
From: Daniel Jacobowitz @ 2008-03-24 22:39 UTC (permalink / raw)
  To: Nick Roberts; +Cc: Pedro Alves, gdb-patches

On Tue, Mar 25, 2008 at 10:04:01AM +1200, Nick Roberts wrote:
> I think sometimes errors are just needed when debugging the frontend, e.g.,
> 
>   -thread-select
>   ^error,msg="mi_cmd_thread_select: USAGE: threadnum."
>   (gdb) 
> 
> where one error message is sufficient.  Here I think Gdb is trying to mimic
> perror by giving the name of the C procedure, but I think it's not desirable as
> this gets printed due to a frontend error, not a Gdb error.  So I would like to
> see:
> 
>   -thread-select
>   ^error,msg="USAGE: threadnum."
>   (gdb) 

I agree.

> At other times duplicated error messages are desirable, e.g.,
> 
>   -exec-continue 
>   ^running
>   (gdb) 
>   &"The program is not being run.\n"
>   ^error,msg="The program is not being run."
> 
> because the first goes to the console for the user to see, the second to
> the frontend to be handled as appropriate.

Could you explain why the duplication here is good?  It seems to me
that if the front end wants to display this error to the user in the
console, it can do so anyway, but it shouldn't have to.  Maybe this
requires a version bump, I don't know.

In Eclipse, this duplication leads to a bunch of error messages that
pop up in the console pane which don't come from anything the user
did.  It's very confusing.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-24 22:39   ` Daniel Jacobowitz
@ 2008-03-24 23:37     ` Nick Roberts
  2008-03-25  1:00       ` Daniel Jacobowitz
  0 siblings, 1 reply; 26+ messages in thread
From: Nick Roberts @ 2008-03-24 23:37 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Pedro Alves, gdb-patches

 > > because the first goes to the console for the user to see, the second to
 > > the frontend to be handled as appropriate.
 > 
 > Could you explain why the duplication here is good?  It seems to me
 > that if the front end wants to display this error to the user in the
 > console, it can do so anyway, but it shouldn't have to.  Maybe this
 > requires a version bump, I don't know.

Emacs isn't currently using this output, so I'm not speaking from experience,
but how does the front end determine which messages are for the user?

Currently most messages that are due to frontend errors go through error ()
so I guess I'm suggesting the opposite change!

 > In Eclipse, this duplication leads to a bunch of error messages that
 > pop up in the console pane which don't come from anything the user
 > did.  It's very confusing.

Just as you say if the "frontend wants to display this error to the user in
the console, it can do so anyway" isn't it equally true that if it doesn't want
to display this error, it can choose not to so?

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-24 23:37     ` Nick Roberts
@ 2008-03-25  1:00       ` Daniel Jacobowitz
  2008-03-25  1:29         ` Nick Roberts
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel Jacobowitz @ 2008-03-25  1:00 UTC (permalink / raw)
  To: Nick Roberts; +Cc: Pedro Alves, gdb-patches

On Tue, Mar 25, 2008 at 11:37:00AM +1200, Nick Roberts wrote:
> Emacs isn't currently using this output, so I'm not speaking from experience,
> but how does the front end determine which messages are for the user?
> 
> Currently most messages that are due to frontend errors go through error ()
> so I guess I'm suggesting the opposite change!

Any time you call error, it should already be caught and show up in
the ^error output.  The GDB/MI documentation says that ~"" is for:

`"~" STRING-OUTPUT'
     The console output stream contains text that should be displayed
     in the CLI console window.  It contains the textual responses to
     CLI commands.

> Just as you say if the "frontend wants to display this error to the user in
> the console, it can do so anyway" isn't it equally true that if it doesn't want
> to display this error, it can choose not to so?

Yes, but unlike a formatted MI response, the front end doesn't know
what a random bit of ~"output" is.  It might be a notification, like a
new thread, or an error message, or...

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-25  1:00       ` Daniel Jacobowitz
@ 2008-03-25  1:29         ` Nick Roberts
  2008-03-25  2:12           ` Daniel Jacobowitz
  0 siblings, 1 reply; 26+ messages in thread
From: Nick Roberts @ 2008-03-25  1:29 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Pedro Alves, gdb-patches

 > > Just as you say if the "frontend wants to display this error to the user
 > > in the console, it can do so anyway" isn't it equally true that if it
 > > doesn't want to display this error, it can choose not to so?
 > 
 > Yes, but unlike a formatted MI response, the front end doesn't know
 > what a random bit of ~"output" is.  It might be a notification, like a
 > new thread, or an error message, or...

Errors aren't CONSOLE-STREAM-OUTPUT but LOG-STREAM-OUTPUT and are prefixed
with `&' not `~':

&"The program is not being run.\n"
^error,msg="The program is not being run."
(gdb)

=thread-created,id=2
~"[New Thread 0xb7568b90 (LWP 19810)]\n"

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-25  1:29         ` Nick Roberts
@ 2008-03-25  2:12           ` Daniel Jacobowitz
  2008-03-29 14:22             ` Vladimir Prus
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel Jacobowitz @ 2008-03-25  2:12 UTC (permalink / raw)
  To: Nick Roberts; +Cc: Pedro Alves, gdb-patches

On Tue, Mar 25, 2008 at 01:28:26PM +1200, Nick Roberts wrote:
>  > > Just as you say if the "frontend wants to display this error to the user
>  > > in the console, it can do so anyway" isn't it equally true that if it
>  > > doesn't want to display this error, it can choose not to so?
>  > 
>  > Yes, but unlike a formatted MI response, the front end doesn't know
>  > what a random bit of ~"output" is.  It might be a notification, like a
>  > new thread, or an error message, or...
> 
> Errors aren't CONSOLE-STREAM-OUTPUT but LOG-STREAM-OUTPUT and are prefixed
> with `&' not `~':
> 
> &"The program is not being run.\n"
> ^error,msg="The program is not being run."
> (gdb)
> 
> =thread-created,id=2
> ~"[New Thread 0xb7568b90 (LWP 19810)]\n"

Whoops, you're right.  Sorry for confusing the issue.

`"&" STRING-OUTPUT'
     The log stream contains debugging messages being produced by GDB's
     internals.

I still think that means we shouldn't be producing them for
errors.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-24 22:04 ` Nick Roberts
  2008-03-24 22:39   ` Daniel Jacobowitz
@ 2008-03-25  3:52   ` Pedro Alves
  1 sibling, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2008-03-25  3:52 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

A Monday 24 March 2008 22:04:01, Nick Roberts wrote:
>  > Notice the duplication:
>  > &"The program is not being run.\n"
>  > ^error,msg="The program is not being run."
>  >
>
> The error message isn't duplicated when MI_CMD_ERROR is used, but the bulk
> of this patch is about removing it.  Duplication comes from the single
> line:
>
>      exception_print (gdb_stderr, result);
>

Yes.  Sorry I didn't explain that right.  I really wrote the patch after
learning about the mi_error_message ugliness.  We already have an error
mechanism, we don't need another.

> in mi_execute_command.
>
> I think sometimes errors are just needed when debugging the frontend, e.g.,
>
>   -thread-select
>   ^error,msg="mi_cmd_thread_select: USAGE: threadnum."
>   (gdb)
>
> where one error message is sufficient.  Here I think Gdb is trying to mimic
> perror by giving the name of the C procedure, but I think it's not
> desirable as this gets printed due to a frontend error, not a Gdb error. 
> So I would like to see:
>
>   -thread-select
>   ^error,msg="USAGE: threadnum."
>   (gdb)
>

I agree with this, and was postponing doing that until after this went in.

But, I don't understand what's the benefit of printing to the error stream
in some cases an not in others.  If this is supposed to not be seen by
the user normally, and principle is seen only by a frontend developer,
why not make it consistent with the other errors?

> At other times duplicated error messages are desirable, e.g.,
>
>   -exec-continue
>   ^running
>   (gdb)
>   &"The program is not being run.\n"
>   ^error,msg="The program is not being run."

That comes from inflow.c.  There are other cases that aren't
currently consistent, like:

333-var-create int * int
&"Attempt to use a type name as an expression.\n"
&"mi_cmd_var_create: unable to create variable object\n"
333^error,msg="mi_cmd_var_create: unable to create variable object"
(gdb)

Looks like the same kind of error that you wouldn't want
output to &", but it is...
Shouldn't the "Attempt to..." line also go to ^error?

And, look at mi-cmd-var.c for example, 

enum mi_cmd_result
mi_cmd_var_create (char *command, char **argv, int argc)
{
  CORE_ADDR frameaddr = 0;
  struct varobj *var;
  char *name;
  char *frame;
  char *expr;
  struct cleanup *old_cleanups;
  enum varobj_type var_type;

  if (argc != 3)
    {
      /* mi_error_message = xstrprintf ("mi_cmd_var_create: Usage:
         ...."); return MI_CMD_ERROR; */
      error (_("mi_cmd_var_create: Usage: NAME FRAME EXPRESSION."));
    }

In that file, for example, usage errors are output with error.
In mi-main.c you'll see that they aren't...  And this dates back
to the first revision of the files in public CVS.

Looks like the conversion to error had already started ages ago.
Was MI initialy developed outside of gdb?

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-25  2:12           ` Daniel Jacobowitz
@ 2008-03-29 14:22             ` Vladimir Prus
  2008-03-30  5:13               ` Nick Roberts
  0 siblings, 1 reply; 26+ messages in thread
From: Vladimir Prus @ 2008-03-29 14:22 UTC (permalink / raw)
  To: gdb-patches

Daniel Jacobowitz wrote:

> On Tue, Mar 25, 2008 at 01:28:26PM +1200, Nick Roberts wrote:
>>  > > Just as you say if the "frontend wants to display this error to the user
>>  > > in the console, it can do so anyway" isn't it equally true that if it
>>  > > doesn't want to display this error, it can choose not to so?
>>  > 
>>  > Yes, but unlike a formatted MI response, the front end doesn't know
>>  > what a random bit of ~"output" is.  It might be a notification, like a
>>  > new thread, or an error message, or...
>> 
>> Errors aren't CONSOLE-STREAM-OUTPUT but LOG-STREAM-OUTPUT and are prefixed
>> with `&' not `~':
>> 
>> &"The program is not being run.\n"
>> ^error,msg="The program is not being run."
>> (gdb)
>> 
>> =thread-created,id=2
>> ~"[New Thread 0xb7568b90 (LWP 19810)]\n"
> 
> Whoops, you're right.  Sorry for confusing the issue.
> 
> `"&" STRING-OUTPUT'
>      The log stream contains debugging messages being produced by GDB's
>      internals.
> 
> I still think that means we shouldn't be producing them for
> errors.

I agree. What happens now is that on errors, Eclipse shows the error message
in red. Good, however since it does not show the MI commands user will
know what something is wrong, but won't have any clue why. If the point is
just to show to the user that some error has happened, then the ^error is pretty
sufficient. And if we enable logging of MI commands, user can see the
information even if nothing goes to "&" channel.

So I'd agree that removing those error messages is good. I plan to review
the patch in detail next week.

- Volodya

 



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-29 14:22             ` Vladimir Prus
@ 2008-03-30  5:13               ` Nick Roberts
  2008-03-30  6:06                 ` Vladimir Prus
  0 siblings, 1 reply; 26+ messages in thread
From: Nick Roberts @ 2008-03-30  5:13 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > > `"&" STRING-OUTPUT'
 > >      The log stream contains debugging messages being produced by GDB's
 > >      internals.
 > > 
 > > I still think that means we shouldn't be producing them for
 > > errors.

From mi-interp.c:

  /* Create MI channels */
  mi->err = mi_console_file_new (raw_stdout, "&", '"');
  mi->log = mi->err;

Maybe the documentation is imprecise but it's pretty clear from the code that
this channel was intended for errors.

 > I agree. What happens now is that on errors, Eclipse shows the error message
 > in red. Good, however since it does not show the MI commands user will know
 > what something is wrong, but won't have any clue why.

Maybe that's a shortcoming of Eclipse.  And if it is a problem then it is
surely one which an Eclipse developer should raise.

 >                                                       If the point is just
 > to show to the user that some error has happened, then the ^error is pretty
 > sufficient. 

I've said why I don't think it's sufficient but I'm not going to repeat myself.

 >            And if we enable logging of MI commands, user can see the
 > information even if nothing goes to "&" channel.

And nothing goes to the console.  Same old story.

 > So I'd agree that removing those error messages is good. I plan to review
 > the patch in detail next week.


-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-30  5:13               ` Nick Roberts
@ 2008-03-30  6:06                 ` Vladimir Prus
  2008-03-31  0:46                   ` Nick Roberts
  0 siblings, 1 reply; 26+ messages in thread
From: Vladimir Prus @ 2008-03-30  6:06 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Sunday 30 March 2008 09:09:02 Nick Roberts wrote:
>  > > `"&" STRING-OUTPUT'
>  > >      The log stream contains debugging messages being produced by GDB's
>  > >      internals.
>  > > 
>  > > I still think that means we shouldn't be producing them for
>  > > errors.
> 
> From mi-interp.c:
> 
>   /* Create MI channels */
>   mi->err = mi_console_file_new (raw_stdout, "&", '"');
>   mi->log = mi->err;
> 
> Maybe the documentation is imprecise but it's pretty clear from the code that
> this channel was intended for errors.
> 
>  > I agree. What happens now is that on errors, Eclipse shows the error message
>  > in red. Good, however since it does not show the MI commands user will know
>  > what something is wrong, but won't have any clue why.
> 
> Maybe that's a shortcoming of Eclipse.  And if it is a problem then it is
> surely one which an Eclipse developer should raise.

I don't see any possible UI where errors produced from MI commands that
are not shown make any sense.

> 
>  >                                                       If the point is just
>  > to show to the user that some error has happened, then the ^error is pretty
>  > sufficient. 
> 
> I've said why I don't think it's sufficient but I'm not going to repeat myself.

The only thing from you I can find is:

  At other times duplicated error messages are desirable, e.g.,

  -exec-continue 
  ^running
  (gdb) 
  &"The program is not being run.\n"
  ^error,msg="The program is not being run."

  because the first goes to the console for the user to see, the second to
  the frontend to be handled as appropriate.

You still did not say why showing the error message is console is desirable.
If -exec-continue itself is now show in the console, the error message
makes no sense. If -exec-continue is shown, then the error message is not
necessary. Where the flaw in this logic.

> 
>  >            And if we enable logging of MI commands, user can see the
>  > information even if nothing goes to "&" channel.
> 
> And nothing goes to the console.  Same old story.

Sorry, I don't understand what is "old story" and why nothing goes to console.
Both KDevelop and Eclipse will send both MI commands and responses to the console
window if "verbose" mode is enabled.

- Volodya


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-30  6:06                 ` Vladimir Prus
@ 2008-03-31  0:46                   ` Nick Roberts
  2008-03-31  1:59                     ` Daniel Jacobowitz
                                       ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Nick Roberts @ 2008-03-31  0:46 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > The only thing from you I can find is:
 > 
 >   At other times duplicated error messages are desirable, e.g.,
 > 
 >   -exec-continue 
 >   ^running
 >   (gdb) 
 >   &"The program is not being run.\n"
 >   ^error,msg="The program is not being run."
 > 
 >   because the first goes to the console for the user to see, the second to
 >   the frontend to be handled as appropriate.
 > 
 > You still did not say why showing the error message is console is desirable.
 > If -exec-continue itself is now show in the console, the error message
 > makes no sense. If -exec-continue is shown, then the error message is not
 > necessary. Where the flaw in this logic.

I can't understand these sentences.  The command -exec-continue won't appear in
the console but "The program is not being run." will.  These `errors' and other
similar ones like "No stack." are reported through error () and are normal Gdb
output for the user to see.  Currently the console can display such messages by
reading LOG-STREAM-OUTPUT.

Other errors like:

(gdb) 
-interpreter-exec
^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command"
(gdb)

would be due to a frontend error, so I think it would be probably be best to
display them elsewhare, e.g., status bar.

The only way for the frontend to distinguish between the two types of error is
if the Gdb developer uses the appropriate mechanism, i.e. error () or
mi_error_message in each case.

I wouldn't make any changes until a real problem is reported (not just Pedro
tidying things up).  If a change has to be made I would suggest the one below.
However this would mean going through all the errors reported in MI to work out
which ones need mi_error_message but currently use error (), e.g.,
"mi_cmd_stack_list_locals: Usage: PRINT_VALUES".

 > > 
 > >  >            And if we enable logging of MI commands, user can see the
 > >  > information even if nothing goes to "&" channel.
 > > 
 > > And nothing goes to the console.  Same old story.
 > 
 > Sorry, I don't understand what is "old story" and why nothing goes to
 > console.

Right from when I started and there was an attempt to remove annotations,
it feels like the console has been under threat.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


*** mi-main.c	31 Mar 2008 12:08:10 +1200	1.110
--- mi-main.c	31 Mar 2008 12:16:01 +1200	
*************** mi_execute_command (char *cmd, int from_
*** 1260,1278 ****
  	  mi_parse_free (command);
  	  return;
  	}
!       if (result.reason < 0)
! 	{
! 	  /* The command execution failed and error() was called
! 	     somewhere.  */
! 	  fputs_unfiltered (command->token, raw_stdout);
! 	  fputs_unfiltered ("^error,msg=\"", raw_stdout);
! 	  if (result.message == NULL)
! 	    fputs_unfiltered ("unknown error", raw_stdout);
! 	  else
! 	      fputstr_unfiltered (result.message, '"', raw_stdout);
! 	  fputs_unfiltered ("\"\n", raw_stdout);
! 	  mi_out_rewind (uiout);
! 	}
        mi_parse_free (command);
      }
  
--- 1260,1266 ----
  	  mi_parse_free (command);
  	  return;
  	}
! 
        mi_parse_free (command);
      }
  


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-31  0:46                   ` Nick Roberts
@ 2008-03-31  1:59                     ` Daniel Jacobowitz
  2008-03-31  2:23                     ` Nick Roberts
                                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Daniel Jacobowitz @ 2008-03-31  1:59 UTC (permalink / raw)
  To: Nick Roberts; +Cc: Vladimir Prus, gdb-patches

On Mon, Mar 31, 2008 at 12:45:18PM +1200, Nick Roberts wrote:
> Right from when I started and there was an attempt to remove annotations,
> it feels like the console has been under threat.

The console is not under threat.  Heck, I want the console to be ten
times as useful in Eclipse as it is today and I hope we can get there
in the next few years.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-31  0:46                   ` Nick Roberts
  2008-03-31  1:59                     ` Daniel Jacobowitz
@ 2008-03-31  2:23                     ` Nick Roberts
  2008-03-31  5:07                     ` Vladimir Prus
  2008-04-01  2:00                     ` Pedro Alves
  3 siblings, 0 replies; 26+ messages in thread
From: Nick Roberts @ 2008-03-31  2:23 UTC (permalink / raw)
  To: Vladimir Prus, gdb-patches

 >  >   At other times duplicated error messages are desirable, e.g.,
 >  > 
 >  >   -exec-continue 
 >  >   ^running
 >  >   (gdb) 
 >  >   &"The program is not being run.\n"
 >  >   ^error,msg="The program is not being run."

As an aside, emitting ^running looks inappropriate/premature here.

 > If a change has to be made I would suggest the one below.

Perhaps not quite this but with a ^done RESULT-RECORD too.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-31  0:46                   ` Nick Roberts
  2008-03-31  1:59                     ` Daniel Jacobowitz
  2008-03-31  2:23                     ` Nick Roberts
@ 2008-03-31  5:07                     ` Vladimir Prus
  2008-03-31  6:36                       ` Nick Roberts
  2008-04-01  2:00                     ` Pedro Alves
  3 siblings, 1 reply; 26+ messages in thread
From: Vladimir Prus @ 2008-03-31  5:07 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Monday 31 March 2008 04:45:18 Nick Roberts wrote:
>  > The only thing from you I can find is:
>  > 
>  >   At other times duplicated error messages are desirable, e.g.,
>  > 
>  >   -exec-continue 
>  >   ^running
>  >   (gdb) 
>  >   &"The program is not being run.\n"
>  >   ^error,msg="The program is not being run."
>  > 
>  >   because the first goes to the console for the user to see, the second to
>  >   the frontend to be handled as appropriate.
>  > 
>  > You still did not say why showing the error message is console is desirable.
>  > If -exec-continue itself is now show in the console, the error message
>  > makes no sense. If -exec-continue is shown, then the error message is not
>  > necessary. Where the flaw in this logic.
> 
> I can't understand these sentences.  The command -exec-continue won't appear in
> the console but "The program is not being run." will.  

And what is the possible value of that message to the user? It does not correspond
to anything user has typed in, and it does not correspond to anything user sees.
It's just a message out of blue sky.

If we want the console to be truly useful, it should accept all commands from the user,
and print GDB responses, including errors, to those commands. Printing responses to
commands implicitly emitted by the frontend does not seem to be the purpose of
the console window.

> These `errors' and other 
> similar ones like "No stack." are reported through error () and are normal Gdb
> output for the user to see.  Currently the console can display such messages by
> reading LOG-STREAM-OUTPUT.
> 
> Other errors like:
> 
> (gdb) 
> -interpreter-exec
> ^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command"
> (gdb)
> 
> would be due to a frontend error, so I think it would be probably be best to
> display them elsewhare, e.g., status bar.

No proposal to change the ^error is made.

> The only way for the frontend to distinguish between the two types of error is
> if the Gdb developer uses the appropriate mechanism, i.e. error () or
> mi_error_message in each case.
> 
> I wouldn't make any changes until a real problem is reported (not just Pedro
> tidying things up).

You make it sound as if code cleanup is something on N-th priority. I don't think
it's so, it's one of the most important things in current GDB, and it also allows
us to identify nonobvious behaviour, such as happened here.

> If a change has to be made I would suggest the one below. 
> However this would mean going through all the errors reported in MI to work out
> which ones need mi_error_message but currently use error (), e.g.,
> "mi_cmd_stack_list_locals: Usage: PRINT_VALUES".

> *** mi-main.c   31 Mar 2008 12:08:10 +1200      1.110
> --- mi-main.c   31 Mar 2008 12:16:01 +1200      
> *************** mi_execute_command (char *cmd, int from_
> *** 1260,1278 ****
>           mi_parse_free (command);
>           return;
>         }
> !       if (result.reason < 0)
> !       {
> !         /* The command execution failed and error() was called
> !            somewhere.  */
> !         fputs_unfiltered (command->token, raw_stdout);
> !         fputs_unfiltered ("^error,msg=\"", raw_stdout);
> !         if (result.message == NULL)
> !           fputs_unfiltered ("unknown error", raw_stdout);
> !         else
> !             fputstr_unfiltered (result.message, '"', raw_stdout);
> !         fputs_unfiltered ("\"\n", raw_stdout);
> !         mi_out_rewind (uiout);
> !       }
>         mi_parse_free (command);
>       }
>  
> --- 1260,1266 ----
>           mi_parse_free (command);
>           return;
>         }
> !
>         mi_parse_free (command);
>       }
> Perhaps not quite this but with a ^done RESULT-RECORD too.

So, you are suggesting that if execution of MI command results in an call to 'error',
we respond with ^done? That does not appear right to me. Surely, an exception thrown
during processing of a command means we cannot finish that command, which should be
reported to the user.

- Volodya


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-31  5:07                     ` Vladimir Prus
@ 2008-03-31  6:36                       ` Nick Roberts
  2008-03-31  7:10                         ` Vladimir Prus
  2008-03-31 15:17                         ` Pedro Alves
  0 siblings, 2 replies; 26+ messages in thread
From: Nick Roberts @ 2008-03-31  6:36 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > > I can't understand these sentences.  The command -exec-continue won't
 > > appear in the console but "The program is not being run." will.
 > 
 > And what is the possible value of that message to the user? It does not
 > correspond to anything user has typed in, and it does not correspond to
 > anything user sees.  It's just a message out of blue sky.

It's not out of the blue, it's a consequence of user input, just not typed
user input.

 > If we want the console to be truly useful, it should accept all commands
 > from the user, and print GDB responses, including errors, to those
 > commands. Printing responses to commands implicitly emitted by the frontend
 > does not seem to be the purpose of the console window.

I think it's useful.

 > > These `errors' and other similar ones like "No stack." are reported
 > > through error () and are normal Gdb output for the user to see.  Currently
 > > the console can display such messages by reading LOG-STREAM-OUTPUT.
 > > 
 > > Other errors like:
 > > 
 > > (gdb) 
 > > -interpreter-exec
 > > ^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command"
 > > (gdb)
 > > 
 > > would be due to a frontend error, so I think it would be probably be best
 > > to display them elsewhare, e.g., status bar.
 > 
 > No proposal to change the ^error is made.
 
No, but you can't distinguish between the errors with only one channel.

 > > The only way for the frontend to distinguish between the two types of
 > > error is if the Gdb developer uses the appropriate mechanism, i.e. error
 > > () or mi_error_message in each case.
 > > 
 > > I wouldn't make any changes until a real problem is reported (not just
 > > Pedro tidying things up).
 > 
 > You make it sound as if code cleanup is something on N-th priority. I don't
 > think it's so, it's one of the most important things in current GDB, and it
 > also allows us to identify nonobvious behaviour, such as happened here.

My point is that since the code may be of value (the author has gone to
some trouble to create two error channels) and as it's not causing much
inconvenience it's best to leave things as they are for the moment.  I
could give a long list of things that are more important to work on.

 > > If a change has to be made I would suggest the one below.  However this
 > > would mean going through all the errors reported in MI to work out which
 > > ones need mi_error_message but currently use error (), e.g.,
 > > "mi_cmd_stack_list_locals: Usage: PRINT_VALUES".
 > 
 > > *** mi-main.c 31 Mar 2008 12:08:10 +1200 1.110
 > > --- mi-main.c 31 Mar 2008 12:16:01 +1200
 > > *************** mi_execute_command (char *cmd, int from_
 > > *** 1260,1278 ****
 > > ...
 > >
 > > Perhaps not quite this but with a ^done RESULT-RECORD too.
 > 
 > So, you are suggesting that if execution of MI command results in an call to
 > 'error', we respond with ^done? That does not appear right to me. Surely, an
 > exception thrown during processing of a command means we cannot finish that
 > command, which should be reported to the user.

The error is still reported, but on the other channel.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-31  6:36                       ` Nick Roberts
@ 2008-03-31  7:10                         ` Vladimir Prus
  2008-03-31 15:17                         ` Pedro Alves
  1 sibling, 0 replies; 26+ messages in thread
From: Vladimir Prus @ 2008-03-31  7:10 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Monday 31 March 2008 10:35:31 Nick Roberts wrote:
>  > > I can't understand these sentences.  The command -exec-continue won't
>  > > appear in the console but "The program is not being run." will.
>  > 
>  > And what is the possible value of that message to the user? It does not
>  > correspond to anything user has typed in, and it does not correspond to
>  > anything user sees.  It's just a message out of blue sky.
> 
> It's not out of the blue, it's a consequence of user input, just not typed
> user input.

The "user input" was not done in the console window. It was done by pressing
some buttons in UI. It makes sense that any feedback to said input is done by
updating UI where the input was made.

 >  > > The only way for the frontend to distinguish between the two types of
>  > > error is if the Gdb developer uses the appropriate mechanism, i.e. error
>  > > () or mi_error_message in each case.
>  > > 
>  > > I wouldn't make any changes until a real problem is reported (not just
>  > > Pedro tidying things up).
>  > 
>  > You make it sound as if code cleanup is something on N-th priority. I don't
>  > think it's so, it's one of the most important things in current GDB, and it
>  > also allows us to identify nonobvious behaviour, such as happened here.
> 
> My point is that since the code may be of value (the author has gone to
> some trouble to create two error channels) and as it's not causing much
> inconvenience it's best to leave things as they are for the moment.  I
> could give a long list of things that are more important to work on.

As it happens, we already have a patch. 

>  > > If a change has to be made I would suggest the one below.  However this
>  > > would mean going through all the errors reported in MI to work out which
>  > > ones need mi_error_message but currently use error (), e.g.,
>  > > "mi_cmd_stack_list_locals: Usage: PRINT_VALUES".
>  > 
>  > > *** mi-main.c 31 Mar 2008 12:08:10 +1200 1.110
>  > > --- mi-main.c 31 Mar 2008 12:16:01 +1200
>  > > *************** mi_execute_command (char *cmd, int from_
>  > > *** 1260,1278 ****
>  > > ...
>  > >
>  > > Perhaps not quite this but with a ^done RESULT-RECORD too.
>  > 
>  > So, you are suggesting that if execution of MI command results in an call to
>  > 'error', we respond with ^done? That does not appear right to me. Surely, an
>  > exception thrown during processing of a command means we cannot finish that
>  > command, which should be reported to the user.
> 
> The error is still reported, but on the other channel.

So, are you suggesting that the frontend, in order to understand if the command
was successful, goes and parses the "&" channel? First, this will totally break
existing frontends. Second, "^error" is clear statement that an error has occurred.
Parsing "&" to guess if an error was occurred is an ad-hoc solution -- that's how
CLI-based frontends used to work, and that was a total mess.

Notice that the message appears at "&" channel right now only because in MI,
both gdb_stdlog and gdb_stderr map to the same channel. Presumably, it's because
the original author did not see much value in separate error channel. Note also
that messages normally output to the gdb_stdlog are for debugging GDB purposes,
like output of "set debug target 1". Notice that also many use of gdb_stderr
are debugging in the purposes, too -- those are messages that don't abort
the current operation but might be useful, if something goes wrong. So, it's
reasonable to assume that the "&" channel is primarily for logging/debugging
purposes.

Then, sending the error that is already reported via other mechanism to the "&"
channel does not seem necessary.

- Volodya








^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-31 15:17                         ` Pedro Alves
@ 2008-03-31 11:24                           ` Pedro Alves
  0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2008-03-31 11:24 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nick Roberts, Vladimir Prus, gdb-patches

A Monday 31 March 2008 07:35:31, Nick Roberts wrote:
>  > > I can't understand these sentences.  The command -exec-continue won't
>  > > appear in the console but "The program is not being run." will.
>  >
>  > And what is the possible value of that message to the user? It does not
>  > correspond to anything user has typed in, and it does not correspond to
>  > anything user sees.  It's just a message out of blue sky.
>
> It's not out of the blue, it's a consequence of user input, just not typed
> user input.
>
>  > If we want the console to be truly useful, it should accept all commands
>  > from the user, and print GDB responses, including errors, to those
>  > commands. Printing responses to commands implicitly emitted by the
>  > frontend does not seem to be the purpose of the console window.
>
> I think it's useful.
>
>  > > These `errors' and other similar ones like "No stack." are reported
>  > > through error () and are normal Gdb output for the user to see. 
>  > > Currently the console can display such messages by reading
>  > > LOG-STREAM-OUTPUT.
>  > >
>  > > Other errors like:
>  > >
>  > > (gdb)
>  > > -interpreter-exec
>  > > ^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp
>  > > command" (gdb)
>  > >
>  > > would be due to a frontend error, so I think it would be probably be
>  > > best to display them elsewhare, e.g., status bar.
>  >
>  > No proposal to change the ^error is made.
>
> No, but you can't distinguish between the errors with only one channel.
>
>  > > The only way for the frontend to distinguish between the two types of
>  > > error is if the Gdb developer uses the appropriate mechanism, i.e.
>  > > error () or mi_error_message in each case.
>  > >
>  > > I wouldn't make any changes until a real problem is reported (not just
>  > > Pedro tidying things up).
>  >
>  > You make it sound as if code cleanup is something on N-th priority. I
>  > don't think it's so, it's one of the most important things in current
>  > GDB, and it also allows us to identify nonobvious behaviour, such as
>  > happened here.
>
> My point is that since the code may be of value (the author has gone to
> some trouble to create two error channels) and as it's not causing much
> inconvenience it's best to leave things as they are for the moment.  I
> could give a long list of things that are more important to work on.
>

If we want to distinguish them, the correct form is to encode it in the 
exception itself.  We can add a new MI_GENERIC_ERROR to "enum errors",
and come up with a new mi_error function that throws that error class
instead of GENERIC_ERROR.  Then we can catch those instead of
handling MI_CMD_ERROR in a pass-error-code-in-return-of-function style.

In fact, my first approach went this path.  I then reverted that into
using generic `error' when I couldn't come up with a valid use for
having some cases output to stderr, and others not.  I've ended up
making everything go to ^error, but if the discussion ends up deciding
things should go to stderr too, I still didn't see a convincing reason
why *some* responses shouldn't go to stderr.

I know that these errors can only come from the frontend passing
invalid args to gdb, but, how often does that happen, that we need
to make sure the user doesn't see it (in the current form of things).  And
if the frontend is failing to coordinate with gdb, why do we go
to great lengths to not tell the user looking at the console?  I'm
not advocating to put things on stderr -- heck, I was the one proposing
not to, I just trying to understand why *you* think that *some* errors,
which normally should *never* happen, should be given *special* treatment.
Honest, I'd like to know that.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-31  6:36                       ` Nick Roberts
  2008-03-31  7:10                         ` Vladimir Prus
@ 2008-03-31 15:17                         ` Pedro Alves
  2008-03-31 11:24                           ` Pedro Alves
  1 sibling, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2008-03-31 15:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nick Roberts, Vladimir Prus, gdb-patches

A Monday 31 March 2008 07:35:31, Nick Roberts wrote:
>  > > I can't understand these sentences.  The command -exec-continue won't
>  > > appear in the console but "The program is not being run." will.
>  >
>  > And what is the possible value of that message to the user? It does not
>  > correspond to anything user has typed in, and it does not correspond to
>  > anything user sees.  It's just a message out of blue sky.
>
> It's not out of the blue, it's a consequence of user input, just not typed
> user input.
>
>  > If we want the console to be truly useful, it should accept all commands
>  > from the user, and print GDB responses, including errors, to those
>  > commands. Printing responses to commands implicitly emitted by the
>  > frontend does not seem to be the purpose of the console window.
>
> I think it's useful.
>
>  > > These `errors' and other similar ones like "No stack." are reported
>  > > through error () and are normal Gdb output for the user to see. 
>  > > Currently the console can display such messages by reading
>  > > LOG-STREAM-OUTPUT.
>  > >
>  > > Other errors like:
>  > >
>  > > (gdb)
>  > > -interpreter-exec
>  > > ^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp
>  > > command" (gdb)
>  > >
>  > > would be due to a frontend error, so I think it would be probably be
>  > > best to display them elsewhare, e.g., status bar.
>  >
>  > No proposal to change the ^error is made.
>
> No, but you can't distinguish between the errors with only one channel.
>
>  > > The only way for the frontend to distinguish between the two types of
>  > > error is if the Gdb developer uses the appropriate mechanism, i.e.
>  > > error () or mi_error_message in each case.
>  > >
>  > > I wouldn't make any changes until a real problem is reported (not just
>  > > Pedro tidying things up).
>  >
>  > You make it sound as if code cleanup is something on N-th priority. I
>  > don't think it's so, it's one of the most important things in current
>  > GDB, and it also allows us to identify nonobvious behaviour, such as
>  > happened here.
>
> My point is that since the code may be of value (the author has gone to
> some trouble to create two error channels) and as it's not causing much
> inconvenience it's best to leave things as they are for the moment.  I
> could give a long list of things that are more important to work on.
>

If we want to distinguish them, the correct form is to encode it in the 
exception itself.  We can add a new MI_GENERIC_ERROR to "enum errors",
and come up with a new mi_error function that throws that error class
instead of GENERIC_ERROR.  Then we can catch those instead of
handling MI_CMD_ERROR in a pass-error-code-in-return-of-function style.

In fact, my first approach went this path.  I then reverted that into
using generic `error' when I couldn't come up with a valid use for
having some cases output to stderr, and others not.  I've ended up
making everything go to ^error, but if the discussion ends up deciding
things should go to stderr too, I still didn't see a convincing reason
why *some* responses shouldn't go to stderr.

I know that these errors can only come from the frontend passing
invalid args to gdb, but, how often does that happen, that we need
to make sure the user doesn't see it (in the current form of things).  And
if the frontend is failing to coordinate with gdb, why do we go
to great lengths to not tell the user looking at the console?  I'm
not advocating to put things on stderr -- heck, I was the one proposing
not to, I just trying to understand why *you* think that *some* errors,
which normally should *never* happen, should be given *special* treatment.
Honest, I'd like to know that.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-04-01  2:00                     ` Pedro Alves
@ 2008-04-01  0:18                       ` Pedro Alves
  2008-04-01 13:17                       ` Nick Roberts
  1 sibling, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2008-04-01  0:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nick Roberts, Vladimir Prus, gdb-patches

A Monday 31 March 2008 01:45:18, Nick Roberts wrote:
>  > The only thing from you I can find is:
>  >
>  >   At other times duplicated error messages are desirable, e.g.,
>  >
>  >   -exec-continue
>  >   ^running
>  >   (gdb)
>  >   &"The program is not being run.\n"
>  >   ^error,msg="The program is not being run."
>  >
>  >   because the first goes to the console for the user to see, the second
>  > to the frontend to be handled as appropriate.
>  >
>  > You still did not say why showing the error message is console is
>  > desirable. If -exec-continue itself is now show in the console, the
>  > error message makes no sense. If -exec-continue is shown, then the error
>  > message is not necessary. Where the flaw in this logic.
>
> I can't understand these sentences.  The command -exec-continue won't
> appear in the console but "The program is not being run." will.  These
> `errors' and other similar ones like "No stack." are reported through error
> () and are normal Gdb output for the user to see.  Currently the console
> can display such messages by reading LOG-STREAM-OUTPUT.
>
> Other errors like:
>
> (gdb)
> -interpreter-exec
> ^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp
> command" (gdb)
>
> would be due to a frontend error, so I think it would be probably be best
> to display them elsewhare, e.g., status bar.
>

I think the screen should flash and beep three times, and an email
should be automatically sent the the frontend author :-)  Seriously,
the status bar doesn't feel right for this.  This is not status.  This
is GDB complaining to the frontend -- which means the frontend is broken
and isn't doing what the user wants to.  A message box with a
"submit bug report" button is in order.  If the frontend wants to
probe for gdb version by trying parameters to a command, then, if
all output goes to ^error, and the frontend uses tokens, it is
easy to selectively not display those errors.

> The only way for the frontend to distinguish between the two types of error
> is if the Gdb developer uses the appropriate mechanism, i.e. error () or
> mi_error_message in each case.
>

Then the proper way would be to either add a new ^mi-error, or add
an error class to the output of error, like ^error,class="critical".
Although, I don't advocate the a for this.

> I wouldn't make any changes until a real problem is reported (not just
> Pedro tidying things up).  If a change has to be made I would suggest the
> one below. However this would mean going through all the errors reported in
> MI to work out which ones need mi_error_message but currently use error (),
> e.g., "mi_cmd_stack_list_locals: Usage: PRINT_VALUES".
>

The log stream output doesn't have any token associated, so as we move
to non-stop mode and multi-process debugging, you can't associate
the log stream output with a command, unless the frontend is waiting
for every command to complete before issuing the next -- possibly
to another process or thread.  My point of view is that, all
errors comming from gdb for a particular command should be output
to ^error, and should be collected in a batch and output in one
MI line only.  We can arrange for the those messages that are only
output to & currently to show up in in ^error channel too.  E.g.:

Today:

 333-var-create int * int
 &"Attempt to use a type name as an expression.\n"
 &"mi_cmd_var_create: unable to create variable object\n"
 333^error,msg="mi_cmd_var_create: unable to create variable object"
 (gdb)

Should be:

 333-var-create int * int
 333^error,msg="Unable to create variable object.\nAttempt to use a type name 
as an expression.\n"
 (gdb)

Or, extending MI:

 333-var-create int * int
 333^error,msg="Unable to create variable object.",reason="Attempt to use a  
type name as an expression.\n"
 (gdb)

If the frontend/user wants, it can put the msg in from ^error,msg= in the 
console too, so there's no loss.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-31  0:46                   ` Nick Roberts
                                       ` (2 preceding siblings ...)
  2008-03-31  5:07                     ` Vladimir Prus
@ 2008-04-01  2:00                     ` Pedro Alves
  2008-04-01  0:18                       ` Pedro Alves
  2008-04-01 13:17                       ` Nick Roberts
  3 siblings, 2 replies; 26+ messages in thread
From: Pedro Alves @ 2008-04-01  2:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nick Roberts, Vladimir Prus, gdb-patches

A Monday 31 March 2008 01:45:18, Nick Roberts wrote:
>  > The only thing from you I can find is:
>  >
>  >   At other times duplicated error messages are desirable, e.g.,
>  >
>  >   -exec-continue
>  >   ^running
>  >   (gdb)
>  >   &"The program is not being run.\n"
>  >   ^error,msg="The program is not being run."
>  >
>  >   because the first goes to the console for the user to see, the second
>  > to the frontend to be handled as appropriate.
>  >
>  > You still did not say why showing the error message is console is
>  > desirable. If -exec-continue itself is now show in the console, the
>  > error message makes no sense. If -exec-continue is shown, then the error
>  > message is not necessary. Where the flaw in this logic.
>
> I can't understand these sentences.  The command -exec-continue won't
> appear in the console but "The program is not being run." will.  These
> `errors' and other similar ones like "No stack." are reported through error
> () and are normal Gdb output for the user to see.  Currently the console
> can display such messages by reading LOG-STREAM-OUTPUT.
>
> Other errors like:
>
> (gdb)
> -interpreter-exec
> ^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp
> command" (gdb)
>
> would be due to a frontend error, so I think it would be probably be best
> to display them elsewhare, e.g., status bar.
>

I think the screen should flash and beep three times, and an email
should be automatically sent the the frontend author :-)  Seriously,
the status bar doesn't feel right for this.  This is not status.  This
is GDB complaining to the frontend -- which means the frontend is broken
and isn't doing what the user wants to.  A message box with a
"submit bug report" button is in order.  If the frontend wants to
probe for gdb version by trying parameters to a command, then, if
all output goes to ^error, and the frontend uses tokens, it is
easy to selectively not display those errors.

> The only way for the frontend to distinguish between the two types of error
> is if the Gdb developer uses the appropriate mechanism, i.e. error () or
> mi_error_message in each case.
>

Then the proper way would be to either add a new ^mi-error, or add
an error class to the output of error, like ^error,class="critical".
Although, I don't advocate the a for this.

> I wouldn't make any changes until a real problem is reported (not just
> Pedro tidying things up).  If a change has to be made I would suggest the
> one below. However this would mean going through all the errors reported in
> MI to work out which ones need mi_error_message but currently use error (),
> e.g., "mi_cmd_stack_list_locals: Usage: PRINT_VALUES".
>

The log stream output doesn't have any token associated, so as we move
to non-stop mode and multi-process debugging, you can't associate
the log stream output with a command, unless the frontend is waiting
for every command to complete before issuing the next -- possibly
to another process or thread.  My point of view is that, all
errors comming from gdb for a particular command should be output
to ^error, and should be collected in a batch and output in one
MI line only.  We can arrange for the those messages that are only
output to & currently to show up in in ^error channel too.  E.g.:

Today:

 333-var-create int * int
 &"Attempt to use a type name as an expression.\n"
 &"mi_cmd_var_create: unable to create variable object\n"
 333^error,msg="mi_cmd_var_create: unable to create variable object"
 (gdb)

Should be:

 333-var-create int * int
 333^error,msg="Unable to create variable object.\nAttempt to use a type name 
as an expression.\n"
 (gdb)

Or, extending MI:

 333-var-create int * int
 333^error,msg="Unable to create variable object.",reason="Attempt to use a  
type name as an expression.\n"
 (gdb)

If the frontend/user wants, it can put the msg in from ^error,msg= in the 
console too, so there's no loss.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-04-01 13:17                       ` Nick Roberts
@ 2008-04-01  3:28                         ` Nick Roberts
  0 siblings, 0 replies; 26+ messages in thread
From: Nick Roberts @ 2008-04-01  3:28 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Vladimir Prus, gdb-patches

 > > Other errors like:
 > >
 > > (gdb)
 > > -interpreter-exec
 > > ^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp
 > > command" (gdb)
 > >
 > > would be due to a frontend error, so I think it would be probably be best
 > > to display them elsewhare, e.g., status bar.
 > >
 > 
 > I think the screen should flash and beep three times, and an email
 > should be automatically sent the the frontend author :-)  Seriously,
 > the status bar doesn't feel right for this.  This is not status.

Please don't take the status bar example too literally, it was just the first
place I thought of, and is not the focus of my message.

To get a meaningful report probably requires full logging of all the
transactions anyway, which is what I do for Emacs.

In any case I've already surrendered on this issue.

Please redirect any frontend errors using Gdb in Emacs reported on this list to
emacs-devel ;-)

Thanks.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-04-01  2:00                     ` Pedro Alves
  2008-04-01  0:18                       ` Pedro Alves
@ 2008-04-01 13:17                       ` Nick Roberts
  2008-04-01  3:28                         ` Nick Roberts
  1 sibling, 1 reply; 26+ messages in thread
From: Nick Roberts @ 2008-04-01 13:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Vladimir Prus, gdb-patches

 > > Other errors like:
 > >
 > > (gdb)
 > > -interpreter-exec
 > > ^error,msg="mi_cmd_interpreter_exec: Usage: -interpreter-exec interp
 > > command" (gdb)
 > >
 > > would be due to a frontend error, so I think it would be probably be best
 > > to display them elsewhare, e.g., status bar.
 > >
 > 
 > I think the screen should flash and beep three times, and an email
 > should be automatically sent the the frontend author :-)  Seriously,
 > the status bar doesn't feel right for this.  This is not status.

Please don't take the status bar example too literally, it was just the first
place I thought of, and is not the focus of my message.

To get a meaningful report probably requires full logging of all the
transactions anyway, which is what I do for Emacs.

In any case I've already surrendered on this issue.

Please redirect any frontend errors using Gdb in Emacs reported on this list to
emacs-devel ;-)

Thanks.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-03-24 18:30 cleanup mi error message handling Pedro Alves
  2008-03-24 19:08 ` Pedro Alves
  2008-03-24 22:04 ` Nick Roberts
@ 2008-04-04 13:33 ` Vladimir Prus
  2008-04-04 23:08   ` Pedro Alves
  2 siblings, 1 reply; 26+ messages in thread
From: Vladimir Prus @ 2008-04-04 13:33 UTC (permalink / raw)
  To: gdb-patches

Pedro Alves wrote:

> Index: src/gdb/mi/mi-main.c
> ===================================================================
> --- src.orig/gdb/mi/mi-main.c   2008-03-23 19:56:34.000000000 +0000
> +++ src/gdb/mi/mi-main.c        2008-03-23 20:00:02.000000000 +0000
> @@ -96,7 +96,6 @@ static int do_timings = 0;
>  /* The token of the last asynchronous command.  */
>  static char *last_async_command;
>  static char *previous_async_command;
> -char *mi_error_message;
>  
>  extern void _initialize_mi_main (void);
>  static enum mi_cmd_result mi_cmd_execute (struct mi_parse *parse);
> @@ -109,7 +108,7 @@ static void mi_exec_async_cli_cmd_contin
>  
>  static int register_changed_p (int regnum, struct regcache *,
>                                struct regcache *);
> -static int get_register (int regnum, int format);
> +static void get_register (int regnum, int format);
>  
>  /* Command implementations.  FIXME: Is this libgdb?  No.  This is the MI
>     layer that calls libgdb.  Any operation used in the below should be
> @@ -219,10 +218,8 @@ enum mi_cmd_result
>  mi_cmd_exec_interrupt (char *args, int from_tty)
>  {
>    if (!target_executing)
> -    {
> -      mi_error_message = xstrprintf ("mi_cmd_exec_interrupt: Inferior not executing.");
> -      return MI_CMD_ERROR;
> -    }
> +    error ("mi_cmd_exec_interrupt: Inferior not executing.");
> +
>    interrupt_target_command (args, from_tty);
>    if (last_async_command)
>      fputs_unfiltered (last_async_command, raw_stdout);
> @@ -242,38 +239,40 @@ enum mi_cmd_result
>  mi_cmd_thread_select (char *command, char **argv, int argc)
>  {
>    enum gdb_rc rc;
> +  char *mi_error_message;
>  
>    if (argc != 1)
> +    error ("mi_cmd_thread_select: USAGE: threadnum.");
> +
> +  rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
> +
> +  if (rc == GDB_RC_FAIL)
>      {
> -      mi_error_message = xstrprintf ("mi_cmd_thread_select: USAGE: threadnum.");
> -      return MI_CMD_ERROR;
> +      make_cleanup (xfree, mi_error_message);
> +      error ("%s", mi_error_message);

Oh, so first gdb_thread_select catches exception and converts it to string,
and then we throw that string again? Eek, but guess we can fix that with
a separate patch.

This patch is OK. Please be sure to re-run the MI testsuite before checking in,
though, as there were various testsuite changes recently.

Thanks,
Volodya



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: cleanup mi error message handling
  2008-04-04 13:33 ` Vladimir Prus
@ 2008-04-04 23:08   ` Pedro Alves
  0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2008-04-04 23:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Vladimir Prus

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

A Friday 04 April 2008 13:34:07, Vladimir Prus wrote:
> > +
> > +  rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
> > +
> > +  if (rc == GDB_RC_FAIL)
> >      {
> > -      mi_error_message = xstrprintf ("mi_cmd_thread_select: USAGE:
> > threadnum."); -      return MI_CMD_ERROR;
> > +      make_cleanup (xfree, mi_error_message);
> > +      error ("%s", mi_error_message);
>
> Oh, so first gdb_thread_select catches exception and converts it to string,
> and then we throw that string again? Eek, but guess we can fix that with
> a separate patch.
>

I understood this is as one of those libgdb-functions-can't-throw
issues (gdb_ prefix).  I don't know if we should be worrying about
those today, but didn't want to go there for this patch.

> This patch is OK. Please be sure to re-run the MI testsuite before checking
> in, though, as there were various testsuite changes recently.
>

Refreshed, retested, and checked in.  Attached is the patch I applied.

Thanks,
-- 
Pedro Alves

[-- Attachment #2: mi_exceptions.diff --]
[-- Type: text/x-diff, Size: 46643 bytes --]

gdb/
2008-04-04  Pedro Alves  <pedro@codesourcery.com>

	* mi/mi-cmds.h (enum mi_cmd_result): Delete MI_CMD_ERROR.
	(mi_error_message): Delete declaration.
	* mi/mi-interp.c (mi_cmd_interpreter_exec): Call error instead of
	returning MI_CMD_ERROR.
	* mi/mi-main.c (mi_error_message): Delete.
	(mi_cmd_exec_interrupt):
	(mi_cmd_thread_select, mi_cmd_thread_list_ids)
	(mi_cmd_thread_info): Call error instead of returning
	MI_CMD_ERROR.
	(mi_cmd_data_list_register_values): Call error instead of
	returning MI_CMD_ERROR.  Adapt to new get_register interface.
	(get_register): Change return typo to void.  Call error instead of
	returning MI_CMD_ERROR.
	(mi_cmd_data_write_register_values): Call error instead of
	returning MI_CMD_ERROR.
	(mi_cmd_list_features): Return MI_CMD_DONE.
	(captured_mi_execute_command): Remove MI_CMD_ERROR handling.
	(mi_execute_command): Always print exceptions with -error.

gdb/testsuite/
2008-04-04  Pedro Alves  <pedro@codesourcery.com>

	* gdb.mi/mi-disassemble.exp, gdb.mi/mi-stack.exp,
	gdb.mi/mi-syn-frame.exp, gdb.mi/mi-var-block.exp,
	gdb.mi/mi-var-cmd.exp, gdb.mi/mi-var-display.exp,
	gdb.mi/mi2-disassemble.exp, gdb.mi/mi2-stack.exp,
	gdb.mi/mi2-syn-frame.exp, gdb.mi/mi2-var-block.exp,
	gdb.mi/mi2-var-cmd.exp, gdb.mi/mi2-var-display.exp: Update to not
	expect an mi error duplicated in stderr.

---
 gdb/mi/mi-cmds.h                         |    5 
 gdb/mi/mi-interp.c                       |   28 +--
 gdb/mi/mi-main.c                         |  251 +++++++++----------------------
 gdb/testsuite/gdb.mi/mi-disassemble.exp  |    8 
 gdb/testsuite/gdb.mi/mi-stack.exp        |    8 
 gdb/testsuite/gdb.mi/mi-syn-frame.exp    |    7 
 gdb/testsuite/gdb.mi/mi-var-block.exp    |    4 
 gdb/testsuite/gdb.mi/mi-var-cmd.exp      |   10 -
 gdb/testsuite/gdb.mi/mi-var-display.exp  |    2 
 gdb/testsuite/gdb.mi/mi2-disassemble.exp |    8 
 gdb/testsuite/gdb.mi/mi2-stack.exp       |    8 
 gdb/testsuite/gdb.mi/mi2-syn-frame.exp   |    6 
 gdb/testsuite/gdb.mi/mi2-var-block.exp   |    4 
 gdb/testsuite/gdb.mi/mi2-var-cmd.exp     |   10 -
 gdb/testsuite/gdb.mi/mi2-var-display.exp |    2 
 15 files changed, 130 insertions(+), 231 deletions(-)

Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/mi/mi-main.c	2008-04-04 22:54:17.000000000 +0100
@@ -96,7 +96,6 @@ static int do_timings = 0;
 /* The token of the last asynchronous command.  */
 static char *last_async_command;
 static char *previous_async_command;
-char *mi_error_message;
 
 extern void _initialize_mi_main (void);
 static enum mi_cmd_result mi_cmd_execute (struct mi_parse *parse);
@@ -109,7 +108,7 @@ static void mi_exec_async_cli_cmd_contin
 
 static int register_changed_p (int regnum, struct regcache *,
 			       struct regcache *);
-static int get_register (int regnum, int format);
+static void get_register (int regnum, int format);
 
 /* Command implementations.  FIXME: Is this libgdb?  No.  This is the MI
    layer that calls libgdb.  Any operation used in the below should be
@@ -219,10 +218,8 @@ enum mi_cmd_result
 mi_cmd_exec_interrupt (char *args, int from_tty)
 {
   if (!target_executing)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_exec_interrupt: Inferior not executing.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_exec_interrupt: Inferior not executing.");
+
   interrupt_target_command (args, from_tty);
   if (last_async_command)
     fputs_unfiltered (last_async_command, raw_stdout);
@@ -242,38 +239,40 @@ enum mi_cmd_result
 mi_cmd_thread_select (char *command, char **argv, int argc)
 {
   enum gdb_rc rc;
+  char *mi_error_message;
 
   if (argc != 1)
+    error ("mi_cmd_thread_select: USAGE: threadnum.");
+
+  rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
+
+  if (rc == GDB_RC_FAIL)
     {
-      mi_error_message = xstrprintf ("mi_cmd_thread_select: USAGE: threadnum.");
-      return MI_CMD_ERROR;
+      make_cleanup (xfree, mi_error_message);
+      error ("%s", mi_error_message);
     }
-  else
-    rc = gdb_thread_select (uiout, argv[0], &mi_error_message);
 
-  if (rc == GDB_RC_FAIL)
-    return MI_CMD_ERROR;
-  else
-    return MI_CMD_DONE;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
 mi_cmd_thread_list_ids (char *command, char **argv, int argc)
 {
   enum gdb_rc rc;
+  char *mi_error_message;
 
   if (argc != 0)
+    error ("mi_cmd_thread_list_ids: No arguments required.");
+
+  rc = gdb_list_thread_ids (uiout, &mi_error_message);
+
+  if (rc == GDB_RC_FAIL)
     {
-      mi_error_message = xstrprintf ("mi_cmd_thread_list_ids: No arguments required.");
-      return MI_CMD_ERROR;
+      make_cleanup (xfree, mi_error_message);
+      error ("%s", mi_error_message);
     }
-  else
-    rc = gdb_list_thread_ids (uiout, &mi_error_message);
 
-  if (rc == GDB_RC_FAIL)
-    return MI_CMD_ERROR;
-  else
-    return MI_CMD_DONE;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
@@ -282,10 +281,7 @@ mi_cmd_thread_info (char *command, char 
   int thread = -1;
   
   if (argc != 0 && argc != 1)
-    {
-      mi_error_message = xstrprintf ("Invalid MI command");
-      return MI_CMD_ERROR;
-    }
+    error ("Invalid MI command");
 
   if (argc == 1)
     thread = atoi (argv[0]);
@@ -333,11 +329,8 @@ mi_cmd_data_list_register_names (char *c
     {
       regnum = atoi (argv[i]);
       if (regnum < 0 || regnum >= numregs)
-	{
-	  do_cleanups (cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
+
       if (gdbarch_register_name (current_gdbarch, regnum) == NULL
 	  || *(gdbarch_register_name (current_gdbarch, regnum)) == '\0')
 	ui_out_field_string (uiout, NULL, "");
@@ -388,11 +381,7 @@ mi_cmd_data_list_changed_registers (char
 	    continue;
 	  changed = register_changed_p (regnum, prev_regs, this_regs);
 	  if (changed < 0)
-	    {
-	      do_cleanups (cleanup);
-	      mi_error_message = xstrprintf ("mi_cmd_data_list_changed_registers: Unable to read register contents.");
-	      return MI_CMD_ERROR;
-	    }
+	    error ("mi_cmd_data_list_changed_registers: Unable to read register contents.");
 	  else if (changed)
 	    ui_out_field_int (uiout, NULL, regnum);
 	}
@@ -410,20 +399,12 @@ mi_cmd_data_list_changed_registers (char
 	{
 	  changed = register_changed_p (regnum, prev_regs, this_regs);
 	  if (changed < 0)
-	    {
-	      do_cleanups (cleanup);
-	      mi_error_message = xstrprintf ("mi_cmd_data_list_register_change: Unable to read register contents.");
-	      return MI_CMD_ERROR;
-	    }
+	    error ("mi_cmd_data_list_register_change: Unable to read register contents.");
 	  else if (changed)
 	    ui_out_field_int (uiout, NULL, regnum);
 	}
       else
-	{
-	  do_cleanups (cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   do_cleanups (cleanup);
   return MI_CMD_DONE;
@@ -465,7 +446,7 @@ register_changed_p (int regnum, struct r
 enum mi_cmd_result
 mi_cmd_data_list_register_values (char *command, char **argv, int argc)
 {
-  int regnum, numregs, format, result;
+  int regnum, numregs, format;
   int i;
   struct cleanup *list_cleanup, *tuple_cleanup;
 
@@ -479,10 +460,7 @@ mi_cmd_data_list_register_values (char *
 	    + gdbarch_num_pseudo_regs (current_gdbarch);
 
   if (argc == 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: Usage: -data-list-register-values <format> [<regnum1>...<regnumN>]");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_list_register_values: Usage: -data-list-register-values <format> [<regnum1>...<regnumN>]");
 
   format = (int) argv[0][0];
 
@@ -499,12 +477,7 @@ mi_cmd_data_list_register_values (char *
 	    continue;
 	  tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_int (uiout, "number", regnum);
-	  result = get_register (regnum, format);
-	  if (result == -1)
-	    {
-	      do_cleanups (list_cleanup);
-	      return MI_CMD_ERROR;
-	    }
+	  get_register (regnum, format);
 	  do_cleanups (tuple_cleanup);
 	}
     }
@@ -521,27 +494,18 @@ mi_cmd_data_list_register_values (char *
 	{
 	  tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_int (uiout, "number", regnum);
-	  result = get_register (regnum, format);
-	  if (result == -1)
-	    {
-	      do_cleanups (list_cleanup);
-	      return MI_CMD_ERROR;
-	    }
+	  get_register (regnum, format);
 	  do_cleanups (tuple_cleanup);
 	}
       else
-	{
-	  do_cleanups (list_cleanup);
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   do_cleanups (list_cleanup);
   return MI_CMD_DONE;
 }
 
 /* Output one register's contents in the desired format.  */
-static int
+static void
 get_register (int regnum, int format)
 {
   gdb_byte buffer[MAX_REGISTER_SIZE];
@@ -560,10 +524,7 @@ get_register (int regnum, int format)
 		  &realnum, buffer);
 
   if (optim)
-    {
-      mi_error_message = xstrprintf ("Optimized out");
-      return -1;
-    }
+    error ("Optimized out");
 
   if (format == 'r')
     {
@@ -589,7 +550,6 @@ get_register (int regnum, int format)
       ui_out_field_stream (uiout, "value", stb);
       ui_out_stream_delete (stb);
     }
-  return 1;
 }
 
 /* Write given values into registers. The registers and values are
@@ -611,30 +571,18 @@ mi_cmd_data_write_register_values (char 
 	    + gdbarch_num_pseudo_regs (current_gdbarch);
 
   if (argc == 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: Usage: -data-write-register-values <format> [<regnum1> <value1>...<regnumN> <valueN>]");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: Usage: -data-write-register-values <format> [<regnum1> <value1>...<regnumN> <valueN>]");
 
   format = (int) argv[0][0];
 
   if (!target_has_registers)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: No registers.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: No registers.");
 
   if (!(argc - 1))
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: No regs and values specified.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: No regs and values specified.");
 
   if ((argc - 1) % 2)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_register_values: Regs and vals are not in pairs.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_register_values: Regs and vals are not in pairs.");
 
   for (i = 1; i < argc; i = i + 2)
     {
@@ -653,10 +601,7 @@ mi_cmd_data_write_register_values (char 
 	  regcache_cooked_write_signed (get_current_regcache (), regnum, value);
 	}
       else
-	{
-	  mi_error_message = xstrprintf ("bad register number");
-	  return MI_CMD_ERROR;
-	}
+	error ("bad register number");
     }
   return MI_CMD_DONE;
 }
@@ -676,9 +621,8 @@ mi_cmd_data_evaluate_expression (char *c
 
   if (argc != 1)
     {
-      mi_error_message = xstrprintf ("mi_cmd_data_evaluate_expression: Usage: -data-evaluate-expression expression");
       ui_out_stream_delete (stb);
-      return MI_CMD_ERROR;
+      error ("mi_cmd_data_evaluate_expression: Usage: -data-evaluate-expression expression");
     }
 
   expr = parse_expression (argv[0]);
@@ -808,10 +752,7 @@ mi_cmd_data_read_memory (char *command, 
   argc -= optind;
 
   if (argc < 5 || argc > 6)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: Usage: ADDR WORD-FORMAT WORD-SIZE NR-ROWS NR-COLS [ASCHAR].");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: Usage: ADDR WORD-FORMAT WORD-SIZE NR-ROWS NR-COLS [ASCHAR].");
 
   /* Extract all the arguments. */
 
@@ -847,17 +788,13 @@ mi_cmd_data_read_memory (char *command, 
   /* The number of rows.  */
   nr_rows = atol (argv[3]);
   if (nr_rows <= 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: invalid number of rows.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: invalid number of rows.");
+
   /* Number of bytes per row.  */
   nr_cols = atol (argv[4]);
   if (nr_cols <= 0)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_read_memory: invalid number of columns.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_read_memory: invalid number of columns.");
+
   /* The un-printable character when printing ascii.  */
   if (argc == 6)
     aschar = *argv[5];
@@ -872,11 +809,7 @@ mi_cmd_data_read_memory (char *command, 
   nr_bytes = target_read (&current_target, TARGET_OBJECT_MEMORY, NULL,
 			  mbuf, addr, total_bytes);
   if (nr_bytes <= 0)
-    {
-      do_cleanups (cleanups);
-      mi_error_message = xstrdup ("Unable to read memory.");
-      return MI_CMD_ERROR;
-    }
+    error ("Unable to read memory.");
 
   /* Output the header information.  */
   ui_out_field_core_addr (uiout, "addr", addr);
@@ -1008,10 +941,7 @@ mi_cmd_data_write_memory (char *command,
   argc -= optind;
 
   if (argc != 4)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_data_write_memory: Usage: [-o COLUMN_OFFSET] ADDR FORMAT WORD-SIZE VALUE.");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_data_write_memory: Usage: [-o COLUMN_OFFSET] ADDR FORMAT WORD-SIZE VALUE.");
 
   /* Extract all the arguments.  */
   /* Start address of the memory dump.  */
@@ -1060,7 +990,7 @@ mi_cmd_enable_timings (char *command, ch
 
  usage_error:
   error ("mi_cmd_enable_timings: Usage: %s {yes|no}", command);
-  return MI_CMD_ERROR;
+  return MI_CMD_DONE;
 }
 
 enum mi_cmd_result
@@ -1081,7 +1011,7 @@ mi_cmd_list_features (char *command, cha
     }
 
   error ("-list-features should be passed no arguments");
-  return MI_CMD_ERROR;
+  return MI_CMD_DONE;
 }
  
 /* Execute a command within a safe environment.
@@ -1121,7 +1051,7 @@ captured_mi_execute_command (struct ui_o
       args->rc = mi_cmd_execute (context);
 
       if (do_timings)
-          timestamp (&cmd_finished);
+	timestamp (&cmd_finished);
 
       if (!target_can_async_p () || !target_executing)
 	{
@@ -1143,19 +1073,6 @@ captured_mi_execute_command (struct ui_o
 		  print_diff (context->cmd_start, &cmd_finished);
 	      fputs_unfiltered ("\n", raw_stdout);
 	    }
-	  else if (args->rc == MI_CMD_ERROR)
-	    {
-	      if (mi_error_message)
-		{
-		  fputs_unfiltered (context->token, raw_stdout);
-		  fputs_unfiltered ("^error,msg=\"", raw_stdout);
-		  fputstr_unfiltered (mi_error_message, '"', raw_stdout);
-		  xfree (mi_error_message);
-		  mi_error_message = NULL;
-		  fputs_unfiltered ("\"\n", raw_stdout);
-		}
-	      mi_out_rewind (uiout);
-	    }
 	  else
 	    mi_out_rewind (uiout);
 	}
@@ -1196,19 +1113,6 @@ captured_mi_execute_command (struct ui_o
 		fputs_unfiltered ("\n", raw_stdout);
 		args->action = EXECUTE_COMMAND_DISPLAY_PROMPT;
 	      }
-	    else if (args->rc == MI_CMD_ERROR)
-	      {
-		if (mi_error_message)
-		  {
-		    fputs_unfiltered (context->token, raw_stdout);
-		    fputs_unfiltered ("^error,msg=\"", raw_stdout);
-		    fputstr_unfiltered (mi_error_message, '"', raw_stdout);
-		    xfree (mi_error_message);
-		    mi_error_message = NULL;
-		    fputs_unfiltered ("\"\n", raw_stdout);
-		  }
-		mi_out_rewind (uiout);
-	      }
 	    else
 	      mi_out_rewind (uiout);
 	  }
@@ -1246,20 +1150,9 @@ mi_execute_command (char *cmd, int from_
 	  timestamp (command->cmd_start);
 	}
 
-      /* FIXME: cagney/1999-11-04: Can this use of catch_exceptions either
-         be pushed even further down or even eliminated?  */
       args.command = command;
       result = catch_exception (uiout, captured_mi_execute_command, &args,
 				RETURN_MASK_ALL);
-      exception_print (gdb_stderr, result);
-
-      if (args.action == EXECUTE_COMMAND_SUPPRESS_PROMPT)
-	{
-	  /* The command is executing synchronously.  Bail out early
-	     suppressing the finished prompt.  */
-	  mi_parse_free (command);
-	  return;
-	}
       if (result.reason < 0)
 	{
 	  /* The command execution failed and error() was called
@@ -1269,11 +1162,17 @@ mi_execute_command (char *cmd, int from_
 	  if (result.message == NULL)
 	    fputs_unfiltered ("unknown error", raw_stdout);
 	  else
-	      fputstr_unfiltered (result.message, '"', raw_stdout);
+	    fputstr_unfiltered (result.message, '"', raw_stdout);
 	  fputs_unfiltered ("\"\n", raw_stdout);
 	  mi_out_rewind (uiout);
 	}
+
       mi_parse_free (command);
+
+      if (args.action == EXECUTE_COMMAND_SUPPRESS_PROMPT)
+	/* The command is executing synchronously.  Bail out early
+	   suppressing the finished prompt.  */
+	return;
     }
 
   fputs_unfiltered ("(gdb) \n", raw_stdout);
@@ -1311,13 +1210,15 @@ mi_cmd_execute (struct mi_parse *parse)
 	    previous_async_command = xstrdup (last_async_command);
 	  if (strcmp (parse->command, "exec-interrupt"))
 	    {
-	      fputs_unfiltered (parse->token, raw_stdout);
-	      fputs_unfiltered ("^error,msg=\"", raw_stdout);
-	      fputs_unfiltered ("Cannot execute command ", raw_stdout);
-	      fputstr_unfiltered (parse->command, '"', raw_stdout);
-	      fputs_unfiltered (" while target running", raw_stdout);
-	      fputs_unfiltered ("\"\n", raw_stdout);
-	      return MI_CMD_ERROR;
+	      struct ui_file *stb;
+	      stb = mem_fileopen ();
+
+	      fputs_unfiltered ("Cannot execute command ", stb);
+	      fputstr_unfiltered (parse->command, '"', stb);
+	      fputs_unfiltered (" while target running", stb);
+
+	      make_cleanup_ui_file_delete (stb);
+	      error_stream (stb);
 	    }
 	}
       last_async_command = xstrdup (parse->token);
@@ -1339,13 +1240,19 @@ mi_cmd_execute (struct mi_parse *parse)
   else
     {
       /* FIXME: DELETE THIS.  */
-      fputs_unfiltered (parse->token, raw_stdout);
-      fputs_unfiltered ("^error,msg=\"", raw_stdout);
-      fputs_unfiltered ("Undefined mi command: ", raw_stdout);
-      fputstr_unfiltered (parse->command, '"', raw_stdout);
-      fputs_unfiltered (" (missing implementation)", raw_stdout);
-      fputs_unfiltered ("\"\n", raw_stdout);
-      return MI_CMD_ERROR;
+      struct ui_file *stb;
+
+      stb = mem_fileopen ();
+
+      fputs_unfiltered ("Undefined mi command: ", stb);
+      fputstr_unfiltered (parse->command, '"', stb);
+      fputs_unfiltered (" (missing implementation)", stb);
+
+      make_cleanup_ui_file_delete (stb);
+      error_stream (stb);
+
+      /* unreacheable */
+      return MI_CMD_DONE;
     }
 }
 
Index: src/gdb/mi/mi-cmds.h
===================================================================
--- src.orig/gdb/mi/mi-cmds.h	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/mi/mi-cmds.h	2008-04-04 22:54:17.000000000 +0100
@@ -33,10 +33,6 @@ enum mi_cmd_result
     /* The command is still running in the forground.  Main loop should
        display the completion prompt. */
     MI_CMD_FORGROUND,
-    /* An error condition was detected and an error message was
-       asprintf'd into the mi_error_message buffer.  The main loop will
-       display the error message and the completion prompt. */
-    MI_CMD_ERROR,
     /* The MI command has already displayed its completion message.
        Main loop will not display a completion message but will display
        the completion prompt. */
@@ -156,7 +152,6 @@ extern int mi_debug_p;
 /* Raw console output - FIXME: should this be a parameter? */
 extern struct ui_file *raw_stdout;
 
-extern char *mi_error_message;
 extern void mi_execute_command (char *cmd, int from_tty);
 
 #endif
Index: src/gdb/mi/mi-interp.c
===================================================================
--- src.orig/gdb/mi/mi-interp.c	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/mi/mi-interp.c	2008-04-04 22:54:17.000000000 +0100
@@ -190,29 +190,21 @@ enum mi_cmd_result
 mi_cmd_interpreter_exec (char *command, char **argv, int argc)
 {
   struct interp *interp_to_use;
-  enum mi_cmd_result result = MI_CMD_DONE;
   int i;
   struct interp_procs *procs;
+  char *mi_error_message = NULL;
+  struct cleanup *old_chain;
 
   if (argc < 2)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command");
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: Usage: -interpreter-exec interp command");
 
   interp_to_use = interp_lookup (argv[0]);
   if (interp_to_use == NULL)
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: could not find interpreter \"%s\"", argv[0]);
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: could not find interpreter \"%s\"", argv[0]);
 
   if (!interp_exec_p (interp_to_use))
-    {
-      mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: interpreter \"%s\" does not support command execution",
-				     argv[0]);
-      return MI_CMD_ERROR;
-    }
+    error ("mi_cmd_interpreter_exec: interpreter \"%s\" does not support command execution",
+	      argv[0]);
 
   /* Insert the MI out hooks, making sure to also call the interpreter's hooks
      if it has any. */
@@ -222,13 +214,14 @@ mi_cmd_interpreter_exec (char *command, 
 
   /* Now run the code... */
 
+  old_chain = make_cleanup (null_cleanup, 0);
   for (i = 1; i < argc; i++)
     {
       struct gdb_exception e = interp_exec (interp_to_use, argv[i]);
       if (e.reason < 0)
 	{
 	  mi_error_message = xstrdup (e.message);
-	  result = MI_CMD_ERROR;
+	  make_cleanup (xfree, mi_error_message);
 	  break;
 	}
     }
@@ -246,7 +239,10 @@ mi_cmd_interpreter_exec (char *command, 
       add_continuation (mi_interpreter_exec_continuation, NULL);
     }
 
-  return result;
+  if (mi_error_message != NULL)
+    error ("%s", mi_error_message);
+  do_cleanups (old_chain);
+  return MI_CMD_DONE;
 }
 
 /*
Index: src/gdb/testsuite/gdb.mi/mi-disassemble.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-disassemble.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi-disassemble.exp	2008-04-04 22:54:17.000000000 +0100
@@ -160,19 +160,19 @@ proc test_disassembly_bogus_args {} {
     # -data-disassembly -f basics.c -l 32 -- 9
 
     mi_gdb_test "123-data-disassemble -f foo -l abc -n 0 -- 0" \
-             "&.*123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
+             "123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
              "data-disassemble bogus filename"
 
     mi_gdb_test "321-data-disassemble -s foo -e bar -- 0" \
-             "&.*321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
+             "321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
              "data-disassemble bogus address"
 
     mi_gdb_test "456-data-disassemble -s \$pc -f basics.c -- 0" \
-             "&.*456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
+             "456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
              "data-disassemble mix different args"
 
     mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
-             "&.*789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
+             "789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
              "data-disassemble wrong mode arg"
 
 }
Index: src/gdb/testsuite/gdb.mi/mi-stack.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-stack.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi-stack.exp	2008-04-04 22:54:17.000000000 +0100
@@ -69,7 +69,7 @@ proc test_stack_frame_listing {} {
                 "stack frame listing 1 3"
 
     mi_gdb_test "234-stack-list-frames 1" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack frame listing wrong"
 
     mi_gdb_test "235-stack-info-frame" \
@@ -120,7 +120,7 @@ proc test_stack_args_listing {} {
                 "stack args listing 1 1 3"
 
     mi_gdb_test "234-stack-list-arguments" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack args listing wrong"
 
     mi_gdb_test "235-stack-list-arguments 1 1 300" \
@@ -151,7 +151,7 @@ proc test_stack_info_depth {} {
                 "stack info-depth 99"
 
     mi_gdb_test "231-stack-info-depth 99 99" \
-	    "&.*231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
+	    "231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
                 "stack info-depth wrong usage"
 }
 
@@ -190,7 +190,7 @@ gdb_expect {
   "stack locals listing, simple types: names and values, complex type: names and types"
 
     mi_gdb_test "234-stack-list-locals" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
 	    "stack locals listing wrong"
 
     mi_gdb_test "232-stack-select-frame 1" \
Index: src/gdb/testsuite/gdb.mi/mi-var-block.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-block.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi-var-block.exp	2008-04-04 22:54:17.000000000 +0100
@@ -47,7 +47,7 @@ mi_runto do_block_tests
 mi_create_varobj "cb" "cb" "create local variable cb"
 
 mi_gdb_test "-var-create foo * foo" \
-       "&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+       "\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
        "try to create local variable foo"
 
 # step to "foo = 123;"
@@ -58,7 +58,7 @@ mi_step_to "do_block_tests" "" "var-cmd.
 
 # Be paranoid and assume 3.2 created foo
 mi_gdb_test "-var-delete foo" \
-	"&\"Variable object not found\\\\n\".*\\^error,msg=\"Variable object not found\"" \
+	"\\^error,msg=\"Variable object not found\"" \
 	"delete var foo"
 
 
Index: src/gdb/testsuite/gdb.mi/mi-var-cmd.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-cmd.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi-var-cmd.exp	2008-04-04 22:54:17.000000000 +0100
@@ -56,14 +56,14 @@ mi_create_varobj "global_simple" "global
 # Desc: Create non-existent variable
 
 mi_gdb_test "112-var-create bogus_unknown_variable * bogus_unknown_variable" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create non-existent variable"
 
 # Test: c_variable-1.3
 # Desc: Create out of scope variable
 
 mi_gdb_test "113-var-create argc * argc" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create out of scope variable"
 
 mi_runto do_locals_tests
@@ -123,7 +123,7 @@ mi_create_varobj_checked lsimple.integer
 #    Type names (like int, long, etc..) are all proper expressions to gdb.
 #    make sure variable code does not allow users to create variables, though.
 mi_gdb_test "-var-create int * int" \
-	"&\"Attempt to use a type name as an expression.\\\\n\".*&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"&\"Attempt to use a type name as an expression.\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create int"
 
 
@@ -244,7 +244,7 @@ mi_gdb_test "-var-update *" \
 #
 ###
 mi_gdb_test "-var-assign global_simple 0" \
-	"&\"mi_cmd_var_assign: Variable object is not editable\\\\n\".*\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
+	"\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
 	"assign to global_simple"
 
 mi_gdb_test "-var-assign linteger 3333" \
@@ -414,7 +414,7 @@ mi_create_varobj_checked l l {long int \
 # Test: c_variable-2.11
 # Desc: create do_locals_tests local in subroutine1
 mi_gdb_test "-var-create linteger * linteger" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create linteger"
 
 mi_step_to "subroutine1" "\{name=\"i\",value=\".*\"\},\{name=\"l\",value=\".*\"\}" \
Index: src/gdb/testsuite/gdb.mi/mi-var-display.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-var-display.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi-var-display.exp	2008-04-04 22:54:17.000000000 +0100
@@ -544,7 +544,7 @@ mi_gdb_test "-var-evaluate-expression an
 # Test: c_variable-7.70
 # Desc: create anone
 mi_gdb_test "-var-create anone * anone" \
-	"&\"Duplicate variable object name\\\\n\".*\\^error,msg=\"Duplicate variable object name\"" \
+	"\\^error,msg=\"Duplicate variable object name\"" \
 	"create duplicate local variable anone"
 
 
Index: src/gdb/testsuite/gdb.mi/mi2-disassemble.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-disassemble.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi2-disassemble.exp	2008-04-04 22:54:17.000000000 +0100
@@ -160,19 +160,19 @@ proc test_disassembly_bogus_args {} {
     # -data-disassembly -f basics.c -l 32 -- 9
 
     mi_gdb_test "123-data-disassemble -f foo -l abc -n 0 -- 0" \
-             "&.*123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
+             "123\\^error,msg=\"mi_cmd_disassemble: Invalid filename.\"" \
              "data-disassemble bogus filename"
 
     mi_gdb_test "321-data-disassemble -s foo -e bar -- 0" \
-             "&.*321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
+             "321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
              "data-disassemble bogus address"
 
     mi_gdb_test "456-data-disassemble -s \$pc -f basics.c -- 0" \
-             "&.*456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
+             "456\\^error,msg=\"mi_cmd_disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr.\\) .--. mixed_mode.\"" \
              "data-disassemble mix different args"
 
     mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
-             "&.*789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
+             "789\\^error,msg=\"mi_cmd_disassemble: Mixed_mode argument must be 0 or 1.\"" \
              "data-disassemble wrong mode arg"
 
 }
Index: src/gdb/testsuite/gdb.mi/mi2-stack.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-stack.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi2-stack.exp	2008-04-04 22:54:17.000000000 +0100
@@ -69,7 +69,7 @@ proc test_stack_frame_listing {} {
                 "stack frame listing 1 3"
 
     mi_gdb_test "234-stack-list-frames 1" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack frame listing wrong"
 
     mi_gdb_test "235-stack-info-frame" \
@@ -120,7 +120,7 @@ proc test_stack_args_listing {} {
                 "stack args listing 1 1 3"
 
     mi_gdb_test "234-stack-list-arguments" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_args: Usage.*PRINT_VALUES.*FRAME_LOW FRAME_HIGH.*\"" \
 	    "stack args listing wrong"
 
     mi_gdb_test "235-stack-list-arguments 1 1 300" \
@@ -151,7 +151,7 @@ proc test_stack_info_depth {} {
                 "stack info-depth 99"
 
     mi_gdb_test "231-stack-info-depth 99 99" \
-	    "&.*231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
+	    "231\\^error,msg=\"mi_cmd_stack_info_depth: Usage: .MAX_DEPTH.\"" \
                 "stack info-depth wrong usage"
 }
 
@@ -190,7 +190,7 @@ gdb_expect {
   "stack locals listing, simple types: names and values, complex type: names and types"
 
     mi_gdb_test "234-stack-list-locals" \
-	    "&.*234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
+	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
 	    "stack locals listing wrong"
 
     mi_gdb_test "232-stack-select-frame 1" \
Index: src/gdb/testsuite/gdb.mi/mi2-var-block.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-block.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi2-var-block.exp	2008-04-04 22:54:17.000000000 +0100
@@ -47,7 +47,7 @@ mi_runto do_block_tests
 mi_create_varobj "cb" "cb" "create local variable cb"
 
 mi_gdb_test "-var-create foo * foo" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create local variable foo"
 
 # step to "foo = 123;"
@@ -58,7 +58,7 @@ mi_step_to "do_block_tests" "" "var-cmd.
 
 # Be paranoid and assume 3.2 created foo
 mi_gdb_test "-var-delete foo" \
-	"&\"Variable object not found\\\\n\".*\\^error,msg=\"Variable object not found\"" \
+	"\\^error,msg=\"Variable object not found\"" \
 	"delete var foo"
 
 
Index: src/gdb/testsuite/gdb.mi/mi2-var-cmd.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-cmd.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi2-var-cmd.exp	2008-04-04 22:54:17.000000000 +0100
@@ -56,14 +56,14 @@ mi_create_varobj "global_simple" "global
 # Desc: Create non-existent variable
 
 mi_gdb_test "112-var-create bogus_unknown_variable * bogus_unknown_variable" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"112\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create non-existent variable"
 
 # Test: c_variable-1.3
 # Desc: Create out of scope variable
 
 mi_gdb_test "113-var-create argc * argc" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"113\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create out of scope variable"
 
 mi_runto do_locals_tests
@@ -123,7 +123,7 @@ mi_create_varobj_checked lsimple.integer
 #    Type names (like int, long, etc..) are all proper expressions to gdb.
 #    make sure variable code does not allow users to create variables, though.
 mi_gdb_test "-var-create int * int" \
-	"&\"Attempt to use a type name as an expression.\\\\n\".*&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"&\"Attempt to use a type name as an expression.\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create int"
 
 
@@ -244,7 +244,7 @@ mi_gdb_test "-var-update *" \
 #
 ###
 mi_gdb_test "-var-assign global_simple 0" \
-	"&\"mi_cmd_var_assign: Variable object is not editable\\\\n\".*\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
+	"\\^error,msg=\"mi_cmd_var_assign: Variable object is not editable\"" \
 	"assign to global_simple"
 
 mi_gdb_test "-var-assign linteger 3333" \
@@ -377,7 +377,7 @@ mi_create_varobj_checked l l {long int \
 # Test: c_variable-2.11
 # Desc: create do_locals_tests local in subroutine1
 mi_gdb_test "-var-create linteger * linteger" \
-	"&\"mi_cmd_var_create: unable to create variable object\\\\n\".*\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
+	"\\^error,msg=\"mi_cmd_var_create: unable to create variable object\"" \
 	"create linteger"
 
 mi_step_to "subroutine1" "\{name=\"i\",value=\".*\"\},\{name=\"l\",value=\".*\"\}" \
Index: src/gdb/testsuite/gdb.mi/mi2-var-display.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-var-display.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi2-var-display.exp	2008-04-04 22:54:17.000000000 +0100
@@ -544,7 +544,7 @@ mi_gdb_test "-var-evaluate-expression an
 # Test: c_variable-7.70
 # Desc: create anone
 mi_gdb_test "-var-create anone * anone" \
-	"&\"Duplicate variable object name\\\\n\".*\\^error,msg=\"Duplicate variable object name\"" \
+	"\\^error,msg=\"Duplicate variable object name\"" \
 	"create duplicate local variable anone"
 
 
Index: src/gdb/testsuite/gdb.mi/mi-syn-frame.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-syn-frame.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi-syn-frame.exp	2008-04-04 22:54:17.000000000 +0100
@@ -48,7 +48,8 @@ mi_gdb_test "400-break-insert foo" \
 # Call foo() by hand, where we'll hit a breakpoint.
 #
 
-mi_gdb_test "401-data-evaluate-expression foo()" "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(foo\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.*\"" "call inferior's function with a breakpoint set in it"
+mi_gdb_test "401-data-evaluate-expression foo()" "401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(foo\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" "call inferior's function with a breakpoint set in it"
+
 
 mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
 
@@ -75,7 +76,7 @@ mi_gdb_test "405-break-insert subroutine
   "insert breakpoint subroutine"
 
 mi_gdb_test "406-data-evaluate-expression have_a_very_merry_interrupt()" \
-  "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
+  "406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
   "data evaluate expression"
 
 # We should have both a signal handler and a call dummy frame
@@ -99,7 +100,7 @@ mi_gdb_test "409-stack-list-frames 0 0" 
 # 
 
 mi_gdb_test "410-data-evaluate-expression bar()" \
-  "\\&\"The program being debugged was signaled while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"GDB remains in the frame where the signal was received.\\\\n\"\[\r\n\]+\\&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\n\"\[\r\n\]+\\&\"Evaluation of the expression containing the function \\(bar\\) will be abandoned.\\\\n\"\[\r\n\]+410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" \
+  "410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" \
   "call inferior function which raises exception"
 
 mi_gdb_test "411-stack-list-frames" "411\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
Index: src/gdb/testsuite/gdb.mi/mi2-syn-frame.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi2-syn-frame.exp	2008-04-04 22:53:20.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi2-syn-frame.exp	2008-04-04 22:54:17.000000000 +0100
@@ -50,7 +50,7 @@ mi_gdb_test "400-break-insert foo" \
 # Call foo() by hand, where we'll hit a breakpoint.
 #
 
-mi_gdb_test "401-data-evaluate-expression foo()" "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(foo\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.*\"" "call inferior's function with a breakpoint set in it"
+mi_gdb_test "401-data-evaluate-expression foo()" "401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(foo\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" "call inferior's function with a breakpoint set in it"
 
 mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
 
@@ -82,7 +82,7 @@ mi_gdb_test "405-break-insert subroutine
   "insert breakpoint subroutine"
 
 mi_gdb_test "406-data-evaluate-expression have_a_very_merry_interrupt()" \
-  "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
+  "406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
   "evaluate expression have_a_very_merry_interrupt"
 
 # We should have both a signal handler and a call dummy frame
@@ -111,7 +111,7 @@ mi_gdb_test "409-stack-list-frames 0 0" 
 # Call bar() by hand, which should get an exception while running.
 # 
 
-mi_gdb_test "410-data-evaluate-expression bar()" "\\&\"The program being debugged was signaled while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"GDB remains in the frame where the signal was received.\\\\n\"\[\r\n\]+\\&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\n\"\[\r\n\]+\\&\"Evaluation of the expression containing the function \\(bar\\) will be abandoned.\\\\n\"\[\r\n\]+410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" "call inferior function which raises exception"
+mi_gdb_test "410-data-evaluate-expression bar()" "410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" "call inferior function which raises exception"
 
 mi_gdb_test "411-stack-list-frames" "411\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
 

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2008-04-04 22:06 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-24 18:30 cleanup mi error message handling Pedro Alves
2008-03-24 19:08 ` Pedro Alves
2008-03-24 22:04 ` Nick Roberts
2008-03-24 22:39   ` Daniel Jacobowitz
2008-03-24 23:37     ` Nick Roberts
2008-03-25  1:00       ` Daniel Jacobowitz
2008-03-25  1:29         ` Nick Roberts
2008-03-25  2:12           ` Daniel Jacobowitz
2008-03-29 14:22             ` Vladimir Prus
2008-03-30  5:13               ` Nick Roberts
2008-03-30  6:06                 ` Vladimir Prus
2008-03-31  0:46                   ` Nick Roberts
2008-03-31  1:59                     ` Daniel Jacobowitz
2008-03-31  2:23                     ` Nick Roberts
2008-03-31  5:07                     ` Vladimir Prus
2008-03-31  6:36                       ` Nick Roberts
2008-03-31  7:10                         ` Vladimir Prus
2008-03-31 15:17                         ` Pedro Alves
2008-03-31 11:24                           ` Pedro Alves
2008-04-01  2:00                     ` Pedro Alves
2008-04-01  0:18                       ` Pedro Alves
2008-04-01 13:17                       ` Nick Roberts
2008-04-01  3:28                         ` Nick Roberts
2008-03-25  3:52   ` Pedro Alves
2008-04-04 13:33 ` Vladimir Prus
2008-04-04 23:08   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox