Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result...

[Joel Brobecker <brobecker at adacore dot com>]

Note to code reviewers: While working on Eli's comments, I've also
used that opportunity to rename UNKNOWN_COMMAND_ERROR Into
UNDEFINED_COMMAND_ERROR.  Everything should be nicely consistent, now.

> > This error code is only displayed when the corresponding error
> > condition is met. Otherwise, the error record remains unchanged.
> > For instance:
> > 
> >     -symbol-list-lines foo.adb
> >     ^error,msg="-symbol-list-lines: Unknown source file name."
> 
> Doesn't this constitute a reason for bumping the MI revision?

I do not think so, because the change is upward compatible
(consumers are expected to ignore fields they do not handle).

> > +  ** The "^error" result record returned when trying to execute an undefined
> > +     GDB/MI command now provides a variable named "code" whose content is the
> > +     "undefined-command" error code.
> 
> OK, but I would mention the fact that this can be inquired about, as
> you described in your message.

OK. New version attached.

> > +@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
> >  @findex ^error
> > -The operation failed.  The @code{@var{c-string}} contains the corresponding
> > +The operation failed.  The @var{msg} variable contains the corresponding
> >  error message.
> >  
> > +If present, the @var{code} variable provides an error code on which
> 
> The markup is wrong here: "code" is not a variable, it is a literal
> symbol.  You probably meant "c-string" instead.

I've updated both "code" and "msg" (just above). Let me know if it reads
better for you. ("code" is the name of a variable in GDB/MI lexicon).

gdb/ChangeLog:

        (from Pedro Alves  <palves@redhat.com>)
        (from Joel Brobecker  <brobecker@adacore.com>)
        * exceptions.h (enum_errors) <UNDEFINED_COMMAND_ERROR>: New enum.
        * mi/mi-parse.c (mi_parse): Thow UNDEFINED_COMMAND_ERROR instead
        of a regular error when the GDB/MI command does not exist.
        * mi/mi-main.c (mi_cmd_list_features): Add
        "undefined-command-error-code".
        (mi_print_exception): Print an "undefined-command"
        error code if EXCEPTION.ERROR in UNDEFINED_COMMAND_ERROR.
        * NEWS: Add entry documenting the new "code" variable in
        "^error" result records.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
        "^error" result record concerning the error message.  Document
        the error code that may also be part of that result record.
        (GDB/MI Miscellaneous Commands): Document the
        "undefined-command-error-code" element in the output of
        the "-list-features" GDB/MI command.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-undefined-cmd.exp: New testcase.

All retested on x86_64-linux.  OK to check in?

Thank you,
-- 
Joel

>From 5f2f7f77efae56fb3e1bced2cbd504ecd486e8ba Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Mon, 18 Nov 2013 16:55:16 +0400
Subject: [PATCH 2/2] Add "undefined-command" error code at end of ^error
 result...

... when trying to execute an undefined GDB/MI command. When trying
to execute a GDB/MI command which does not exist, the current error
result record looks like this:

    -unsupported
    ^error,msg="Undefined MI command: unsupported"

The only indication that the command does not exist is the error
message. It would be a little fragile for a consumer to rely solely
on the contents of the error message in order to determine whether
a command exists or not.

This patch improves the situation by adding concept of error
code, starting with one well-defined error code ("undefined-command")
identifying errors due to a non-existant command. Here is the new
output:

    -unsupported
    ^error,msg="Undefined MI command: unsupported",code="undefined-command"

This error code is only displayed when the corresponding error
condition is met. Otherwise, the error record remains unchanged.
For instance:

    -symbol-list-lines foo.adb
    ^error,msg="-symbol-list-lines: Unknown source file name."

For frontends to be able to know whether they can rely on this
variable, a new entry "undefined-command-error-code" has been
added to the "-list-features" command.  Another option would be
to always generate an error="..." variable (for the default case,
we could decide for instance that the error code is the empty string).
But it seems more efficient to provide that info in "-list-features"
and then only add the error code when meaningful.

gdb/ChangeLog:

        (from Pedro Alves  <palves@redhat.com>)
        (from Joel Brobecker  <brobecker@adacore.com>)
        * exceptions.h (enum_errors) <UNDEFINED_COMMAND_ERROR>: New enum.
        * mi/mi-parse.c (mi_parse): Thow UNDEFINED_COMMAND_ERROR instead
        of a regular error when the GDB/MI command does not exist.
        * mi/mi-main.c (mi_cmd_list_features): Add
        "undefined-command-error-code".
        (mi_print_exception): Print an "undefined-command"
        error code if EXCEPTION.ERROR in UNDEFINED_COMMAND_ERROR.
        * NEWS: Add entry documenting the new "code" variable in
        "^error" result records.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
        "^error" result record concerning the error message.  Document
        the error code that may also be part of that result record.
        (GDB/MI Miscellaneous Commands): Document the
        "undefined-command-error-code" element in the output of
        the "-list-features" GDB/MI command.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-undefined-cmd.exp: New testcase.
---
 gdb/NEWS                                  |  6 ++++++
 gdb/doc/gdb.texinfo                       | 19 +++++++++++++++---
 gdb/exceptions.h                          |  3 +++
 gdb/mi/mi-main.c                          | 12 ++++++++++-
 gdb/mi/mi-parse.c                         |  3 ++-
 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp | 33 +++++++++++++++++++++++++++++++
 6 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index e61c79f..8ef03d6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -156,6 +156,12 @@ show startup-with-shell
   ** The new command -info-gdb-mi-command allows the user to determine
      whether a GDB/MI command is supported or not.
 
+  ** The "^error" result record returned when trying to execute an undefined
+     GDB/MI command now provides a variable named "code" whose content is the
+     "undefined-command" error code.  Support for this feature can be verified
+     by using the "-list-features" command, which should contain
+     "undefined-command-error-code".
+
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 41856b4..9e86972 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -29315,10 +29315,19 @@ which threads are resumed.
 @findex ^connected
 @value{GDBN} has connected to a remote target.
 
-@item "^error" "," @var{c-string}
+@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
 @findex ^error
-The operation failed.  The @code{@var{c-string}} contains the corresponding
-error message.
+The operation failed.  The @code{msg=@var{c-string}} variable contains
+the corresponding error message.
+
+If present, the @code{code=@var{c-string}} variable provides an error
+code on which consumers can rely in order to detect the corresponding
+error condition.  At present, only one error code is defined:
+
+@table @samp
+@item "undefined-command"
+Indicates that the command causing the error does not exist.
+@end table
 
 @item "^exit"
 @findex ^exit
@@ -35175,6 +35184,10 @@ Indicates that all @sc{gdb/mi} commands accept the @option{--language}
 option (@pxref{Context management}).
 @item info-gdb-mi-command
 Indicates support for the @code{-info-gdb-mi-command} command.
+@item undefined-command-error-code
+Indicates support for the "undefined-command" error code in error result
+records, produced when trying to execute an undefined @sc{gdb/mi} command
+(@pxref{GDB/MI Result Records}).
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 129d29a..a3a28f4 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -93,6 +93,9 @@ enum errors {
      aborted as the inferior state is no longer valid.  */
   TARGET_CLOSE_ERROR,
 
+  /* An undefined command was executed.  */
+  UNDEFINED_COMMAND_ERROR,
+
   /* Add more errors here.  */
   NR_ERRORS
 };
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 48c8d09..3a0e6a8 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1818,6 +1818,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "ada-exceptions");
       ui_out_field_string (uiout, NULL, "language-option");
       ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
+      ui_out_field_string (uiout, NULL, "undefined-command-error-code");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
@@ -2023,7 +2024,16 @@ mi_print_exception (const char *token, struct gdb_exception exception)
     fputs_unfiltered ("unknown error", raw_stdout);
   else
     fputstr_unfiltered (exception.message, '"', raw_stdout);
-  fputs_unfiltered ("\"\n", raw_stdout);
+  fputs_unfiltered ("\"", raw_stdout);
+
+  switch (exception.error)
+    {
+      case UNDEFINED_COMMAND_ERROR:
+	fputs_unfiltered (",code=\"undefined-command\"", raw_stdout);
+	break;
+    }
+
+  fputs_unfiltered ("\n", raw_stdout);
 }
 
 void
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index a2634f1..a092759 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -285,7 +285,8 @@ mi_parse (const char *cmd, char **token)
   /* Find the command in the MI table.  */
   parse->cmd = mi_lookup (parse->command);
   if (parse->cmd == NULL)
-    error (_("Undefined MI command: %s"), parse->command);
+    throw_error (UNDEFINED_COMMAND_ERROR,
+		 _("Undefined MI command: %s"), parse->command);
 
   /* Skip white space following the command.  */
   chp = skip_spaces_const (chp);
diff --git a/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp b/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp
new file mode 100644
index 0000000..8df0a76
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp
@@ -0,0 +1,33 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+
+# First, verify that the debugger correctly advertises support
+# for the "undefined-command" error code...
+mi_gdb_test "-list-features" \
+            "\\^done,features=\\\[.*\"undefined-command-error-code\".*\\\]" \
+            "-list-features should include \"undefined-command-error-code\""
+
+mi_gdb_test "-undefined-command" \
+            "\\^error,.*,code=\"undefined-command\"" \
+            "error code when executing undefined command"
-- 
1.8.1.2