From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7873 invoked by alias); 2 Dec 2013 03:26:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 7863 invoked by uid 89); 2 Dec 2013 03:26:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: rock.gnat.com Received: from Unknown (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 02 Dec 2013 03:26:27 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id EB2C41167DE for ; Sun, 1 Dec 2013 22:26:56 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 4xdWXPKinztD for ; Sun, 1 Dec 2013 22:26:56 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 242721167DD for ; Sun, 1 Dec 2013 22:26:56 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 0D553E141B; Mon, 2 Dec 2013 07:26:15 +0400 (RET) Date: Mon, 02 Dec 2013 03:26:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Message-ID: <20131202032615.GO3114@adacore.com> References: <528631F2.40408@redhat.com> <1384794719-20594-1-git-send-email-brobecker@adacore.com> <1384794719-20594-2-git-send-email-brobecker@adacore.com> <83y54lfwrm.fsf@gnu.org> <20131119041022.GF3481@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20131119041022.GF3481@adacore.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2013-12/txt/msg00007.txt.bz2 Kind request for review. The code itself is fairly straightforward, I think. It's more about making sure that we're defining the right command... Thank you! On Tue, Nov 19, 2013 at 08:10:22AM +0400, Joel Brobecker wrote: > > > I now think that it was indeed the correct choice. Not only does it > > > facilitate implementation (but only marginally), it also is consistent > > > with the current output. For instance, notice how GDB names the command > > > in the following error message: > > > > > > -unsupported > > > ^error,msg="Undefined MI command: unsupported" > > > ^^^^^^^^^^^ > > > (no leading dash) > > > > Your example shows _output_ from MI. By contrast, we are talking > > about _input_. When I send commands to MI, I cannot omit the leading > > dash, so it can be very natural to consider it part of the command. > > > > We don't have to advertise that we support the dash, > > > > > Also, looking at the grammar, the leading dash isn't listed > > > as part of what they call the "operation" > > > > IMO, this line of reasoning makes little sense to users. Grammars are > > for programs, not for people. > > To me, documentation is not an issue. I confess that I remain > unconvinced in this case, especially since this is a command meant > for programs rather than humans, so the risk of using it improperly > is low, given the clear documentation. However, I don't have a strong > opinion, and supporting both forms is pretty easy, so unless someone > strongly objects to allowing the second form, I've just gone ahead and > added it. > > Updated patch attached. And for review convenience, I am also attaching > a diff of the changes I made on top of path #1 (to get to the updated > patch). > > gdb/ChangeLog: > > * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare. > * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function. > * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command. > * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command" > field to output of "-list-features". > > * NEWS: Add entry for new -info-gdb-mi-command. > > gdb/doc/ChangeLog: > > * gdb.texinfo (GDB/MI Miscellaneous Commands): Document > the new -info-gdb-mi-command GDB/MI command. Document > the meaning of "-info-gdb-mi-command" in the output of > -list-features. > > gdb/testsuite/ChangeLog: > > * gdb.mi/mi-i-cmd.exp: New file. > > Re-tested on x86_64-linux. OK to commit? > > Thank you, > -- > Joel > From 5a8ff8bf85f7d455c3f75312e30ec5c1e077ae01 Mon Sep 17 00:00:00 2001 > From: Joel Brobecker > Date: Tue, 12 Nov 2013 14:51:30 +0400 > Subject: [PATCH] New GDB/MI command "-info-gdb-mi-command" > > This patch adds a new GDB/MI command meant for graphical frontends > trying to determine whether a given GDB/MI command exists or not. > > Examples: > > -info-gdb-mi-command unsupported-command > ^done,command={exists="false"} > (gdb) > -info-gdb-mi-command symbol-list-lines > ^done,command={exists="true"} > (gdb) > > At the moment, this is the only piece of information that this > command returns. > > Eventually, and if needed, we can extend it to provide > command-specific pieces of information, such as updates to > the command's syntax since inception. This could become, > for instance: > > -info-gdb-mi-command symbol-list-lines > ^done,command={exists="true",features=[]} > (gdb) > -info-gdb-mi-command catch-assert > ^done,command={exists="true",features=["conditions"]} > > In the first case, it would mean that no extra features, > while in the second, it announces that the -catch-assert > command in this version of the debugger supports a feature > called "condition" - exact semantics to be documented with > combined with the rest of the queried command's documentation. > > But for now, we start small, and only worry about existance. > And to bootstrap the process, I have added an entry in the > output of the -list-features command as well ("info-gdb-mi-command"), > allowing the graphical frontends to go through the following process: > > 1. Send -list-features, collect info from there as before; > 2. Check if the output contains "info-gdb-mi-command". > If it does, then support for various commands can be > queried though -info-gdb-mi-command. Newer commands > will be expected to always be checked via this new > -info-gdb-mi-command. > > gdb/ChangeLog: > > * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare. > * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function. > * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command. > * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command" > field to output of "-list-features". > > * NEWS: Add entry for new -info-gdb-mi-command. > > gdb/doc/ChangeLog: > > * gdb.texinfo (GDB/MI Miscellaneous Commands): Document > the new -info-gdb-mi-command GDB/MI command. Document > the meaning of "-info-gdb-mi-command" in the output of > -list-features. > > gdb/testsuite/ChangeLog: > > * gdb.mi/mi-i-cmd.exp: New file. > --- > gdb/NEWS | 3 +++ > gdb/doc/gdb.texinfo | 53 +++++++++++++++++++++++++++++++++++++++ > gdb/mi/mi-cmd-info.c | 29 +++++++++++++++++++++ > gdb/mi/mi-cmds.c | 1 + > gdb/mi/mi-cmds.h | 1 + > gdb/mi/mi-main.c | 1 + > gdb/testsuite/gdb.mi/mi-i-cmd.exp | 46 +++++++++++++++++++++++++++++++++ > 7 files changed, 134 insertions(+) > create mode 100644 gdb/testsuite/gdb.mi/mi-i-cmd.exp > > diff --git a/gdb/NEWS b/gdb/NEWS > index 9fc3638..e61c79f 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -153,6 +153,9 @@ show startup-with-shell > > ** All MI commands now accept an optional "--language" option. > > + ** The new command -info-gdb-mi-command allows the user to determine > + whether a GDB/MI command is supported or not. > + > ** 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 19e9aa5..41856b4 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -35069,6 +35069,57 @@ default shows this information when you start an interactive session. > (gdb) > @end smallexample > > +@subheading The @code{-info-gdb-mi-command} Command > +@cindex @code{-info-gdb-mi-command} > +@findex -info-gdb-mi-command > + > +@subsubheading Synopsis > + > +@smallexample > + -info-gdb-mi-command @var{cmd_name} > +@end smallexample > + > +Query support for the @sc{gdb/mi} command named @var{cmd_name}. > + > +Note that the dash (@code{-}) starting all @sc{gdb/mi} commands > +is technically not part of the command name (@pxref{GDB/MI Input > +Syntax}), and thus should be omitted in @var{cmd_name}. However, > +for ease of use, this command also accepts the form with the leading > +dash. > + > +@subsubheading @value{GDBN} Command > + > +There is no corresponding @value{GDBN} command. > + > +@subsubheading Result > + > +The result is a tuple. There is currently only one field: > + > +@table @samp > +@item exists > +This field is equal to @code{"true"} if the @sc{gdb/mi} command exists, > +@code{"false"} otherwise. > + > +@end table > + > +@subsubheading Example > + > +Here is an example where the @sc{gdb/mi} command does not exist: > + > +@smallexample > +-info-gdb-mi-command unsupported-command > +^done,command=@{exists="false"@} > +@end smallexample > + > +@noindent > +And here is an example where the @sc{gdb/mi} command is known > +to the debugger: > + > +@smallexample > +-info-gdb-mi-command symbol-list-lines > +^done,command=@{exists="true"@} > +@end smallexample > + > @subheading The @code{-list-features} Command > @findex -list-features > > @@ -35122,6 +35173,8 @@ exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and > @item language-option > 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. > @end table > > @subheading The @code{-list-target-features} Command > diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c > index aa4d210..0fce25a 100644 > --- a/gdb/mi/mi-cmd-info.c > +++ b/gdb/mi/mi-cmd-info.c > @@ -71,6 +71,35 @@ mi_cmd_info_ada_exceptions (char *command, char **argv, int argc) > do_cleanups (old_chain); > } > > +/* Implement the "-info-gdb-mi-command" GDB/MI command. */ > + > +void > +mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc) > +{ > + const char *cmd_name; > + struct mi_cmd *cmd; > + struct ui_out *uiout = current_uiout; > + struct cleanup *old_chain; > + > + /* This command takes exactly one argument. */ > + if (argc != 1) > + error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME")); > + cmd_name = argv[0]; > + > + /* Normally, the command name (aka the "operation" in the GDB/MI > + grammar), does not include the leading '-' (dash). But for > + the user's convenience, allow the user to specify the command > + name to be with or without that leading dash. */ > + if (cmd_name[0] == '-') > + cmd_name++; > + > + cmd = mi_lookup (cmd_name); > + > + old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command"); > + ui_out_field_string (uiout, "exists", cmd != NULL ? "true" : "false"); > + do_cleanups (old_chain); > +} > + > void > mi_cmd_info_os (char *command, char **argv, int argc) > { > diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c > index c536d8a..58a8b89 100644 > --- a/gdb/mi/mi-cmds.c > +++ b/gdb/mi/mi-cmds.c > @@ -125,6 +125,7 @@ static struct mi_cmd mi_cmds[] = > DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set), > DEF_MI_CMD_MI ("inferior-tty-show", mi_cmd_inferior_tty_show), > DEF_MI_CMD_MI ("info-ada-exceptions", mi_cmd_info_ada_exceptions), > + DEF_MI_CMD_MI ("info-gdb-mi-command", mi_cmd_info_gdb_mi_command), > DEF_MI_CMD_MI ("info-os", mi_cmd_info_os), > DEF_MI_CMD_MI ("interpreter-exec", mi_cmd_interpreter_exec), > DEF_MI_CMD_MI ("list-features", mi_cmd_list_features), > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h > index cb8aac1..4ea95fa 100644 > --- a/gdb/mi/mi-cmds.h > +++ b/gdb/mi/mi-cmds.h > @@ -74,6 +74,7 @@ extern mi_cmd_argv_ftype mi_cmd_gdb_exit; > extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set; > extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show; > extern mi_cmd_argv_ftype mi_cmd_info_ada_exceptions; > +extern mi_cmd_argv_ftype mi_cmd_info_gdb_mi_command; > extern mi_cmd_argv_ftype mi_cmd_info_os; > extern mi_cmd_argv_ftype mi_cmd_interpreter_exec; > extern mi_cmd_argv_ftype mi_cmd_list_features; > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index 793204d..48c8d09 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -1817,6 +1817,7 @@ mi_cmd_list_features (char *command, char **argv, int argc) > ui_out_field_string (uiout, NULL, "ada-task-info"); > 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"); > > #if HAVE_PYTHON > if (gdb_python_initialized) > diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp > new file mode 100644 > index 0000000..d460559 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp > @@ -0,0 +1,46 @@ > +# 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 . > + > +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 -info-gdb-mi-command command. > +mi_gdb_test "-list-features" \ > + "\\^done,features=\\\[.*\"info-gdb-mi-command\".*\\\]" \ > + "-list-features should include \"info-gdb-mi-command\"" > + > +mi_gdb_test "-info-gdb-mi-command unsupported-command" \ > + "\\^done,command=\\\{exists=\"false\"\\\}" \ > + "-info-gdb-mi-command unsupported-command" > + > +# Same test as above, but including the leading '-' in the command name. > +mi_gdb_test "-info-gdb-mi-command -unsupported-command" \ > + "\\^done,command=\\\{exists=\"false\"\\\}" \ > + "-info-gdb-mi-command -unsupported-command" > + > +mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \ > + "\\^done,command=\\\{exists=\"true\"\\\}" \ > + "-info-gdb-mi-command symbol-list-lines" > + > +# Same test as above, but including the leading '-' in the command name. > +mi_gdb_test "-info-gdb-mi-command -symbol-list-lines" \ > + "\\^done,command=\\\{exists=\"true\"\\\}" \ > + "-info-gdb-mi-command -symbol-list-lines" > -- > 1.8.1.2 > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index f0662ff..4227f31 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -35088,8 +35088,13 @@ default shows this information when you start an interactive session. > -info-gdb-mi-command @var{cmd_name} > @end smallexample > > -Query support for the @sc{gdb/mi} command named @var{cmd_name} > -(the leading dash (@code{-}) in the command name should be omitted). > +Query support for the @sc{gdb/mi} command named @var{cmd_name}. > + > +Note that the dash (@code{-}) starting all @sc{gdb/mi} commands > +is technically not part of the command name (@pxref{GDB/MI Input > +Syntax}), and thus should be omitted in @var{cmd_name}. However, > +for ease of use, this command also accepts the form with the leading > +dash. > > @subsubheading @value{GDBN} Command > > @@ -35120,6 +35120,7 @@ Here is an example where the @sc{gdb/mi} command does not exist: > ^done,command=@{exists="false"@} > @end smallexample > > +@noindent > And here is an example where the @sc{gdb/mi} command is known > to the debugger: > > diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c > index 18f4927..0fce25a 100644 > --- a/gdb/mi/mi-cmd-info.c > +++ b/gdb/mi/mi-cmd-info.c > @@ -85,6 +85,14 @@ mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc) > if (argc != 1) > error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME")); > cmd_name = argv[0]; > + > + /* Normally, the command name (aka the "operation" in the GDB/MI > + grammar), does not include the leading '-' (dash). But for > + the user's convenience, allow the user to specify the command > + name to be with or without that leading dash. */ > + if (cmd_name[0] == '-') > + cmd_name++; > + > cmd = mi_lookup (cmd_name); > > old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command"); > diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp > index 5285d31..d460559 100644 > --- a/gdb/testsuite/gdb.mi/mi-i-cmd.exp > +++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp > @@ -31,7 +31,16 @@ mi_gdb_test "-info-gdb-mi-command unsupported-command" \ > "\\^done,command=\\\{exists=\"false\"\\\}" \ > "-info-gdb-mi-command unsupported-command" > > +# Same test as above, but including the leading '-' in the command name. > +mi_gdb_test "-info-gdb-mi-command -unsupported-command" \ > + "\\^done,command=\\\{exists=\"false\"\\\}" \ > + "-info-gdb-mi-command -unsupported-command" > + > mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \ > "\\^done,command=\\\{exists=\"true\"\\\}" \ > "-info-gdb-mi-command symbol-list-lines" > > +# Same test as above, but including the leading '-' in the command name. > +mi_gdb_test "-info-gdb-mi-command -symbol-list-lines" \ > + "\\^done,command=\\\{exists=\"true\"\\\}" \ > + "-info-gdb-mi-command -symbol-list-lines" -- Joel