* [RFC] Add ada-exception-catchpoints to -list-features command output.
@ 2013-11-10 17:16 Joel Brobecker
2013-11-10 22:16 ` Eli Zaretskii
2013-11-11 15:22 ` Tom Tromey
0 siblings, 2 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-10 17:16 UTC (permalink / raw)
To: gdb-patches
Hello,
The -list-features GDB/MI command is extremely useful to GDB frontends
who would like to know whether the underlying GDB supports a given
feature or not. Recently, I added new coommands for Ada exception
catching, but forgot about -list-features.
This patch adds an entry meant to help the frontend for those features.
But looking at the way the -list-features command is designed, I am
wondering whether this approach is going to scale well. As new commands
and other new features or major bug fixes get in, it seems like the
list is going to grow maybe a little beyond what's reasonable.
Without trying to redesign this feature entirely, since we're far from
being in that difficult situation, I am thinking it would be OK
to aggregate both fields related to Ada exceptions, namely...
- "info-ada-exceptions" (being introduced as we speak, see
http://www.sourceware.org/ml/gdb-patches/2013-11/msg00232.html)
- this new field (I chose "ada-exceptions-catchpoints")
... into a single field. For instance, we could chose "ada-exceptions",
meant to cover both the commands already in (for catching Ada exceptions),
and the new command being introduced (for listing all Ada exceptions).
So, although this patch proposes a new field (this is the straightforward
approach), given that all this GDB/MI work was done within the same
release cycle, and withing a reasonable amount of time, I think it
would be fine for everyone to use one single field in -list-features.
Thoughts?
In practical terms, I would drop the part in the patch quoted above
that adds the new -list-features field, and would propose another
one, distinct from the "-info-ada-exceptions" patch, which would
then introduce the new "ada-exceptions" -list-features field instead.
Thank you!
gdb/ChangeLog:
* mi/mi-main.c (mi_cmd_list_features): Add
"ada-exception-catchpoints" to -list-features output.
gdb/doc/ChangeLog:
* gdb.texinfo (GDB/MI Miscellaneous Commands): Document new
field "ada-exception-catchpoints" in -list-features output.
---
gdb/doc/gdb.texinfo | 3 +++
gdb/mi/mi-main.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9127f94..9731bbf 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34971,6 +34971,9 @@ CLI will be announced via async records.
indicates support for the @code{-ada-task-info} command.
@item info-ada-exceptions
indicates support for the @code{-info-ada-exceptions} command.
+@item ada-exception-catchpoints
+indicates support for the @code{-catch-assert} and @code{-catch-exception}
+commands.
@end table
@subheading The @code{-list-target-features} Command
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index bf0fce3..8ff7f27 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1816,6 +1816,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
ui_out_field_string (uiout, NULL, "breakpoint-notifications");
ui_out_field_string (uiout, NULL, "ada-task-info");
ui_out_field_string (uiout, NULL, "info-ada-exceptions");
+ ui_out_field_string (uiout, NULL, "ada-exception-catchpoints");
#if HAVE_PYTHON
if (gdb_python_initialized)
--
1.8.1.2
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC] Add ada-exception-catchpoints to -list-features command output. 2013-11-10 17:16 [RFC] Add ada-exception-catchpoints to -list-features command output Joel Brobecker @ 2013-11-10 22:16 ` Eli Zaretskii 2013-11-12 11:25 ` Joel Brobecker 2013-11-11 15:22 ` Tom Tromey 1 sibling, 1 reply; 42+ messages in thread From: Eli Zaretskii @ 2013-11-10 22:16 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > From: Joel Brobecker <brobecker@adacore.com> > Date: Sun, 10 Nov 2013 13:49:20 +0400 > > The -list-features GDB/MI command is extremely useful to GDB frontends > who would like to know whether the underlying GDB supports a given > feature or not. Recently, I added new coommands for Ada exception > catching, but forgot about -list-features. > > This patch adds an entry meant to help the frontend for those features. > But looking at the way the -list-features command is designed, I am > wondering whether this approach is going to scale well. As new commands > and other new features or major bug fixes get in, it seems like the > list is going to grow maybe a little beyond what's reasonable. Thanks, the documentation part is OK. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] Add ada-exception-catchpoints to -list-features command output. 2013-11-10 22:16 ` Eli Zaretskii @ 2013-11-12 11:25 ` Joel Brobecker 2013-11-12 16:39 ` Eli Zaretskii 0 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-11-12 11:25 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 782 bytes --] Hi Eli, > Thanks, the documentation part is OK. Thanks for the review. As discussed with Tom, I've decided to merge the two entries into one. Attached is a patch that does that, together with a revised manual update. Can you tell me what you think? In particular, I am wondering whether the "all related to Ada exceptions" really brings anything in this context other than noise. gdb/ChangeLog: * mi/mi-main.c (mi_cmd_list_features): Replace "info-ada-exceptions" entry with "ada-exceptions". gdb/doc/ChangeLog: * gdb.texinfo (GDB/MI Miscellaneous Commands): Delete the documentation of "info-ada-exceptions" in the output of the "-list-features" command. Add the documentation of the "ada-exception" entry instead. -- Joel [-- Attachment #2: 0001-Replace-info-ada-exceptions-by-ada-exceptions-in-lis.patch --] [-- Type: text/x-diff, Size: 2467 bytes --] From 569128117ccf37ff83033a69929d5c76f1d8acb3 Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Tue, 12 Nov 2013 13:43:44 +0400 Subject: [PATCH] Replace "info-ada-exceptions" by "ada-exceptions" in -list-features Rather than having -list-features report support for the GDB/MI commands providing access to Ada exception catchpoints with one entry, and the GDB/MI command providing the list of Ada exceptions with a second entry, this patch merges it all within one single entry. This is OK, because all these commands were added within a short amount of time, and within the same release cycle; and it reduces a bit the size of the output. gdb/ChangeLog: * mi/mi-main.c (mi_cmd_list_features): Replace "info-ada-exceptions" entry with "ada-exceptions". gdb/doc/ChangeLog: * gdb.texinfo (GDB/MI Miscellaneous Commands): Delete the documentation of "info-ada-exceptions" in the output of the "-list-features" command. Add the documentation of the "ada-exception" entry instead. --- gdb/doc/gdb.texinfo | 6 ++++-- gdb/mi/mi-main.c | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index de419ea..2c4234a 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -35080,8 +35080,10 @@ Indicates that changes to breakpoints and breakpoints created via the CLI will be announced via async records. @item ada-task-info Indicates support for the @code{-ada-task-info} command. -@item info-ada-exceptions -Indicates support for the @code{-info-ada-exceptions} command. +@item ada-exceptions +Indicates support for the following commands, all related to Ada +exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and +@code{-catch-exception}. @end table @subheading The @code{-list-target-features} Command diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index bf0fce3..c3f7221 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1815,7 +1815,7 @@ mi_cmd_list_features (char *command, char **argv, int argc) ui_out_field_string (uiout, NULL, "data-read-memory-bytes"); ui_out_field_string (uiout, NULL, "breakpoint-notifications"); ui_out_field_string (uiout, NULL, "ada-task-info"); - ui_out_field_string (uiout, NULL, "info-ada-exceptions"); + ui_out_field_string (uiout, NULL, "ada-exceptions"); #if HAVE_PYTHON if (gdb_python_initialized) -- 1.8.1.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] Add ada-exception-catchpoints to -list-features command output. 2013-11-12 11:25 ` Joel Brobecker @ 2013-11-12 16:39 ` Eli Zaretskii 2013-11-13 3:02 ` Joel Brobecker 0 siblings, 1 reply; 42+ messages in thread From: Eli Zaretskii @ 2013-11-12 16:39 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > Date: Tue, 12 Nov 2013 13:54:16 +0400 > From: Joel Brobecker <brobecker@adacore.com> > Cc: gdb-patches@sourceware.org > > Thanks for the review. As discussed with Tom, I've decided to > merge the two entries into one. Attached is a patch that does that, > together with a revised manual update. Can you tell me what you think? > In particular, I am wondering whether the "all related to Ada > exceptions" really brings anything in this context other than noise. Looks good to me, although I suggest a minor correction below. > +Indicates support for the following commands, all related to Ada > +exceptions: ^^^^^^^^^^^ I suggest "all of them related" here. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] Add ada-exception-catchpoints to -list-features command output. 2013-11-12 16:39 ` Eli Zaretskii @ 2013-11-13 3:02 ` Joel Brobecker 0 siblings, 0 replies; 42+ messages in thread From: Joel Brobecker @ 2013-11-13 3:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 691 bytes --] > > Thanks for the review. As discussed with Tom, I've decided to > > merge the two entries into one. Attached is a patch that does that, > > together with a revised manual update. Can you tell me what you think? > > In particular, I am wondering whether the "all related to Ada > > exceptions" really brings anything in this context other than noise. > > Looks good to me, although I suggest a minor correction below. > > > +Indicates support for the following commands, all related to Ada > > +exceptions: ^^^^^^^^^^^ > > I suggest "all of them related" here. I made the correction you suggested, and pushed the attached patch. Thank you! -- Joel [-- Attachment #2: 0001-Replace-info-ada-exceptions-by-ada-exceptions-in-lis.patch --] [-- Type: text/x-diff, Size: 3602 bytes --] From 93973826c486a6b7b03eddb201e3ce994dd0fb5b Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Tue, 12 Nov 2013 13:43:44 +0400 Subject: [PATCH] Replace "info-ada-exceptions" by "ada-exceptions" in -list-features Rather than having -list-features report support for the GDB/MI commands providing access to Ada exception catchpoints with one entry, and the GDB/MI command providing the list of Ada exceptions with a second entry, this patch merges it all within one single entry. This is OK, because all these commands were added within a short amount of time, and within the same release cycle; and it reduces a bit the size of the output. gdb/ChangeLog: * mi/mi-main.c (mi_cmd_list_features): Replace "info-ada-exceptions" entry with "ada-exceptions". gdb/doc/ChangeLog: * gdb.texinfo (GDB/MI Miscellaneous Commands): Delete the documentation of "info-ada-exceptions" in the output of the "-list-features" command. Add the documentation of the "ada-exception" entry instead. --- gdb/ChangeLog | 5 +++++ gdb/doc/ChangeLog | 8 ++++++++ gdb/doc/gdb.texinfo | 6 ++++-- gdb/mi/mi-main.c | 2 +- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 279e3e2..6c20cce 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,10 @@ 2013-11-13 Joel Brobecker <brobecker@adacore.com> + * mi/mi-main.c (mi_cmd_list_features): Replace "info-ada-exceptions" + entry with "ada-exceptions". + +2013-11-13 Joel Brobecker <brobecker@adacore.com> + * symfile.c (reread_symbols): Move call to set_objfile_per_bfd after re-initialization of OBJFILE's obstack. diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index 2b13843..968201e 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,5 +1,13 @@ +2013-11-13 Joel Brobecker <brobecker@adacore.com> + + * gdb.texinfo (GDB/MI Miscellaneous Commands): Delete + the documentation of "info-ada-exceptions" in the output + of the "-list-features" command. Add the documentation + of the "ada-exception" entry instead. + 2013-11-12 Joel Brobecker <brobecker@adacore.com> + * gdb.texinfo (GDB/MI Miscellaneous Commands): Fix the first word of a couple of sentences to start with a capital letter. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index de419ea..84acd5c 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -35080,8 +35080,10 @@ Indicates that changes to breakpoints and breakpoints created via the CLI will be announced via async records. @item ada-task-info Indicates support for the @code{-ada-task-info} command. -@item info-ada-exceptions -Indicates support for the @code{-info-ada-exceptions} command. +@item ada-exceptions +Indicates support for the following commands, all of them related to Ada +exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and +@code{-catch-exception}. @end table @subheading The @code{-list-target-features} Command diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index bf0fce3..c3f7221 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1815,7 +1815,7 @@ mi_cmd_list_features (char *command, char **argv, int argc) ui_out_field_string (uiout, NULL, "data-read-memory-bytes"); ui_out_field_string (uiout, NULL, "breakpoint-notifications"); ui_out_field_string (uiout, NULL, "ada-task-info"); - ui_out_field_string (uiout, NULL, "info-ada-exceptions"); + ui_out_field_string (uiout, NULL, "ada-exceptions"); #if HAVE_PYTHON if (gdb_python_initialized) -- 1.8.1.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] Add ada-exception-catchpoints to -list-features command output. 2013-11-10 17:16 [RFC] Add ada-exception-catchpoints to -list-features command output Joel Brobecker 2013-11-10 22:16 ` Eli Zaretskii @ 2013-11-11 15:22 ` Tom Tromey 2013-11-12 9:18 ` Joel Brobecker 2013-11-12 12:11 ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker 1 sibling, 2 replies; 42+ messages in thread From: Tom Tromey @ 2013-11-11 15:22 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes: Joel> This patch adds an entry meant to help the frontend for those features. Joel> But looking at the way the -list-features command is designed, I am Joel> wondering whether this approach is going to scale well. As new commands Joel> and other new features or major bug fixes get in, it seems like the Joel> list is going to grow maybe a little beyond what's reasonable. We could add a way to check for specific commands. Then new commands would never need to be added to the feature list. Joel> So, although this patch proposes a new field (this is the straightforward Joel> approach), given that all this GDB/MI work was done within the same Joel> release cycle, and withing a reasonable amount of time, I think it Joel> would be fine for everyone to use one single field in -list-features. I think it is reasonable, too, provided that the MI docs note the details of what the feature means. Tom ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] Add ada-exception-catchpoints to -list-features command output. 2013-11-11 15:22 ` Tom Tromey @ 2013-11-12 9:18 ` Joel Brobecker 2013-11-12 12:11 ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker 1 sibling, 0 replies; 42+ messages in thread From: Joel Brobecker @ 2013-11-12 9:18 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches > Joel> This patch adds an entry meant to help the frontend for those features. > Joel> But looking at the way the -list-features command is designed, I am > Joel> wondering whether this approach is going to scale well. As new commands > Joel> and other new features or major bug fixes get in, it seems like the > Joel> list is going to grow maybe a little beyond what's reasonable. > > We could add a way to check for specific commands. Then new commands > would never need to be added to the feature list. I like the idea. I need to think a bit about the actual API. I was thinking something along the lines of: -info-gdb-mi-command MI_COMMAND The first implementation would just return one field, which would say yay or nay. But eventually, it might be nice to add other properties, such as maybe some kind of versioning number to help track evolution of the command, or maybe a command-specific feature list. The latter might be less abstract/complex and extensible enough to fit all needs. So, for instance (if the following is valid GDB/MI syntax): -info-gdb-mi-command catch-load -^done,info={exists="true",features=['-c']} which would tell us that the -catch-load command exists, and that in this version of GDB, the "-c" features is supported, for instance telling us that it's possible to add a condition to the catchpoint. > > Joel> So, although this patch proposes a new field (this is the straightforward > Joel> approach), given that all this GDB/MI work was done within the same > Joel> release cycle, and withing a reasonable amount of time, I think it > Joel> would be fine for everyone to use one single field in -list-features. > > I think it is reasonable, too, provided that the MI docs note the > details of what the feature means. Of course. I will send a patch as soon as I have a moment. -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-11 15:22 ` Tom Tromey 2013-11-12 9:18 ` Joel Brobecker @ 2013-11-12 12:11 ` Joel Brobecker 2013-11-12 17:04 ` Eli Zaretskii 2013-11-12 21:17 ` André Pönitz 1 sibling, 2 replies; 42+ messages in thread From: Joel Brobecker @ 2013-11-12 12:11 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches [sorry for the duplicate, Tom, mishandling of the --to= send-email option] Hi Tom, > We could add a way to check for specific commands. Then new commands > would never need to be added to the feature list. What do you think of the following? 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. Tested on x86_64-linux. Comments? Thanks, -- Joel --- gdb/NEWS | 3 +++ gdb/doc/gdb.texinfo | 46 +++++++++++++++++++++++++++++++++++++++ gdb/mi/mi-cmd-info.c | 21 ++++++++++++++++++ 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 | 31 ++++++++++++++++++++++++++ 7 files changed, 104 insertions(+) create mode 100644 gdb/testsuite/gdb.mi/mi-i-cmd.exp diff --git a/gdb/NEWS b/gdb/NEWS index 38209e8..cb1f95a 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -151,6 +151,9 @@ show startup-with-shell * MI changes + ** 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 de419ea..db81ac2 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -35034,6 +35034,50 @@ default shows this information when you start an interactive session. (gdb) @end smallexample +@subheading The @code{-info-gdb-mi-command} Command +@findex -info-gdb-mi-command + +@subsubheading Synopsis + +@smallexample + -info-gdb-mi-command 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). + +@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 + +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 @@ -35082,6 +35126,8 @@ CLI will be announced via async records. Indicates support for the @code{-ada-task-info} command. @item info-ada-exceptions Indicates support for the @code{-info-ada-exceptions} command. +@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..18f4927 100644 --- a/gdb/mi/mi-cmd-info.c +++ b/gdb/mi/mi-cmd-info.c @@ -71,6 +71,27 @@ 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]; + 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 496a8aa..aed62b2 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 bf0fce3..f0846da 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1816,6 +1816,7 @@ mi_cmd_list_features (char *command, char **argv, int argc) ui_out_field_string (uiout, NULL, "breakpoint-notifications"); ui_out_field_string (uiout, NULL, "ada-task-info"); ui_out_field_string (uiout, NULL, "info-ada-exceptions"); + 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..a115451 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp @@ -0,0 +1,31 @@ +# 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 +} + +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" + -- 1.8.1.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-12 12:11 ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker @ 2013-11-12 17:04 ` Eli Zaretskii 2013-11-12 17:48 ` Joel Brobecker 2013-11-12 21:17 ` André Pönitz 1 sibling, 1 reply; 42+ messages in thread From: Eli Zaretskii @ 2013-11-12 17:04 UTC (permalink / raw) To: Joel Brobecker; +Cc: tromey, gdb-patches > From: Joel Brobecker <brobecker@adacore.com> > Cc: gdb-patches@sourceware.org > Date: Tue, 12 Nov 2013 15:25:04 +0400 > > 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) Sounds good to me. > +@subheading The @code{-info-gdb-mi-command} Command > +@findex -info-gdb-mi-command This should be prominently indexed with @cindex entries, as this command is very important, and should be easily found. > +@subsubheading Synopsis > + > +@smallexample > + -info-gdb-mi-command CMD_NAME > +@end smallexample > + > +Query support for the @sc{gdb/mi} command named @var{CMD_NAME} Ts-ts-ts... ASCII art habits die hard. Use @var in the example, and don't upcase CMD_NAME (it is upcased in Info anyway, and will look better in print in lower case). > +(the leading dash (@code{-}) in the command name should be omitted). Is this wise? How about if we support both with and without the dash? > +There is no corresponding @value{GDBN} command. Having a way of querying that in CLI would facilitate better .gdbinit files, I think. > +@smallexample > +-info-gdb-mi-command symbol-list-lines > +^done,command=@{exists="true"@} > +@end smallexample Btw, why "command="? Perhaps "result="? Other than that, the documentation is fine with me. Thanks. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-12 17:04 ` Eli Zaretskii @ 2013-11-12 17:48 ` Joel Brobecker 2013-11-12 18:34 ` Eli Zaretskii 0 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-11-12 17:48 UTC (permalink / raw) To: Eli Zaretskii; +Cc: tromey, gdb-patches > > 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) > > Sounds good to me. > > > +@subheading The @code{-info-gdb-mi-command} Command > > +@findex -info-gdb-mi-command > > This should be prominently indexed with @cindex entries, as this > command is very important, and should be easily found. > > > +@subsubheading Synopsis > > + > > +@smallexample > > + -info-gdb-mi-command CMD_NAME > > +@end smallexample > > + > > +Query support for the @sc{gdb/mi} command named @var{CMD_NAME} > > Ts-ts-ts... ASCII art habits die hard. Use @var in the example, and > don't upcase CMD_NAME (it is upcased in Info anyway, and will look > better in print in lower case). Thanks for the documentation review. I will fix them and post a new patch after we confirm the final version of the command. > > +(the leading dash (@code{-}) in the command name should be omitted). > > Is this wise? How about if we support both with and without the dash? It's just easier to program, as this is how commands are stored in GDB and also looked up by the GDB/MI commadn parser. This can be argued as weak justification, and it is, but we don't really need to do any better, IMO. Since this is a command in a mode mostly intended for machines, I didn't see the point in supporting the other version, or both. I still think the current syntax is fine as clearly documented. But I can implement the more natural one instead, if we want. I think providing two modes of operation would be overkill, though. > > +There is no corresponding @value{GDBN} command. > > Having a way of querying that in CLI would facilitate better .gdbinit > files, I think. Can you give some ideas as to how it would be used. I thought this command to be completely irrelevant to CLI, so didn't even start to consider the idea of providing it in CLI. Remember also that you can always execute a GDB/MI command from CLI using "interpreter-exec". > > +@smallexample > > +-info-gdb-mi-command symbol-list-lines > > +^done,command=@{exists="true"@} > > +@end smallexample > > Btw, why "command="? Perhaps "result="? I don't really have an opinion. I eventually selected "command" because the output describing a command, and "command" was also less generic than the first choice I made ("info"). If people think that "result=" is more MI-like, it's commpletely appropriate to make the change. Thanks! -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-12 17:48 ` Joel Brobecker @ 2013-11-12 18:34 ` Eli Zaretskii 2013-11-13 3:19 ` Joel Brobecker 0 siblings, 1 reply; 42+ messages in thread From: Eli Zaretskii @ 2013-11-12 18:34 UTC (permalink / raw) To: Joel Brobecker; +Cc: tromey, gdb-patches > Date: Tue, 12 Nov 2013 21:43:19 +0400 > From: Joel Brobecker <brobecker@adacore.com> > Cc: tromey@redhat.com, gdb-patches@sourceware.org > > > > +There is no corresponding @value{GDBN} command. > > > > Having a way of querying that in CLI would facilitate better .gdbinit > > files, I think. > > Can you give some ideas as to how it would be used. Like this perhaps: if ($_featurep(FOO)) lets-use-FOO end > Remember also that you can always execute a GDB/MI command from CLI > using "interpreter-exec". But their output is not easily caught and used like above. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-12 18:34 ` Eli Zaretskii @ 2013-11-13 3:19 ` Joel Brobecker 0 siblings, 0 replies; 42+ messages in thread From: Joel Brobecker @ 2013-11-13 3:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: tromey, gdb-patches > > Can you give some ideas as to how it would be used. > > Like this perhaps: > > if ($_featurep(FOO)) > lets-use-FOO > end > > > Remember also that you can always execute a GDB/MI command from CLI > > using "interpreter-exec". > > But their output is not easily caught and used like above. This is true. But I have to say that I am still a little unsure whether this CLI feature will actually be used, should we implement it. Can we wait for an actual request before we do so, especially for a query about GDB/MI when in CLI mode? That way, we then know that we provide something people actually need. If you know you would use that feature now, then of course that qualifies... I think this feature can also be treated independently from the new GDB/MI command. Adding this new feature wouldn't affect this patch, I would think. -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-12 12:11 ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker 2013-11-12 17:04 ` Eli Zaretskii @ 2013-11-12 21:17 ` André Pönitz 2013-11-13 2:47 ` Joel Brobecker 1 sibling, 1 reply; 42+ messages in thread From: André Pönitz @ 2013-11-12 21:17 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches On Tue, Nov 12, 2013 at 03:25:04PM +0400, Joel Brobecker wrote: > [sorry for the duplicate, Tom, mishandling of the --to= send-email option] > > Hi Tom, > > > We could add a way to check for specific commands. Then new commands > > would never need to be added to the feature list. > > What do you think of the following? > > 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) I am not sure I agree with the judgement of benefits here. The basic yes/no information is already there: (gdb) -unsupported-command ^error,msg="Undefined MI command: unsupported-command" (gdb) -symbol-list-lines ^error,msg="-symbol-list-lines: Usage: SOURCE_FILENAME" It's not nice, but "works". In addition, a yes-or-no is not even what might be needed. Look e.g. at the "python" advertisement in -list-features output ^done,features=["frozen-varobjs","pending-breakpoints","thread-info", "data-read-memory-bytes","breakpoint-notifications","ada-task-info","python"] It does not indicate whether it is properly installed (datadir...) nor whether the version of Python is compatible with the script I want to execute. So in practice, checking -list-features is just extra effort giving only a subset of the information I would need for an "ok to use" decision, and it's quicker and more reliable to just execute the command and handle errors. It's hard to imagine that this will ever cover enough of GDB features and questions a frontend needs to have answered. Andre' ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-12 21:17 ` André Pönitz @ 2013-11-13 2:47 ` Joel Brobecker 2013-11-14 0:36 ` André Pönitz 2013-11-14 19:03 ` Pedro Alves 0 siblings, 2 replies; 42+ messages in thread From: Joel Brobecker @ 2013-11-13 2:47 UTC (permalink / raw) To: André Pönitz; +Cc: Tom Tromey, gdb-patches > I am not sure I agree with the judgement of benefits here. The basic > yes/no information is already there: > > (gdb) -unsupported-command > ^error,msg="Undefined MI command: unsupported-command" > (gdb) -symbol-list-lines > ^error,msg="-symbol-list-lines: Usage: SOURCE_FILENAME" > > It's not nice, but "works". I disagree with your assessment of "works". I can think of a number of scenarios where this would be problematic: The first and most obvious to me is the case where the debugger is run with a non-English LANG. If you base your detection on parsing the error msg, then i18n ruins your plan. And if you base your detection on the presence of the error alone, then commands that take no argument may return an error, which by no means indicates that the command does not exist. > In addition, a yes-or-no is not even what might be needed. Well, the IDE team at AdaCore needs that information in order to support the variety of GDB versions out there, and I also agreed with them that this was a sensible request. > Look e.g. at the "python" advertisement in -list-features output > ^done,features=["frozen-varobjs","pending-breakpoints","thread-info", > "data-read-memory-bytes","breakpoint-notifications","ada-task-info","python"] > > It does not indicate whether it is properly installed (datadir...) nor > whether the version of Python is compatible with the script I want to > execute. So in practice, checking -list-features is just extra effort > giving only a subset of the information I would need for an "ok to use" > decision, and it's quicker and more reliable to just execute the command > and handle errors. > > It's hard to imagine that this will ever cover enough of GDB features > and questions a frontend needs to have answered. If we were discussing about the specific issues regarding the use of Python in your example, I would say that this is outside the scope of this new command. If you are trying to make a general point, then can you please tell us how you think we can improve it? If not, you are free to find it useless and to prefer to just use your execution test. But I definitely think it's cleaner to query the debugger with a well documented interface, rather than relying on detecting certain kinds of errors. -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-13 2:47 ` Joel Brobecker @ 2013-11-14 0:36 ` André Pönitz 2013-11-14 9:48 ` Joel Brobecker 2013-11-14 19:03 ` Pedro Alves 1 sibling, 1 reply; 42+ messages in thread From: André Pönitz @ 2013-11-14 0:36 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches On Wed, Nov 13, 2013 at 06:15:14AM +0400, Joel Brobecker wrote: > > I am not sure I agree with the judgement of benefits here. The basic > > yes/no information is already there: > > > > (gdb) -unsupported-command > > ^error,msg="Undefined MI command: unsupported-command" > > (gdb) -symbol-list-lines > > ^error,msg="-symbol-list-lines: Usage: SOURCE_FILENAME" > > > > It's not nice, but "works". > > I disagree with your assessment of "works". I can think of a number > of scenarios where this would be problematic: > > The first and most obvious to me is the case where the debugger is > run with a non-English LANG. If you base your detection on parsing > the error msg, then i18n ruins your plan. For me the context was "frontends". I assume they run external tools in an environment that's as predictable as they need. If a user defined LANG is problematic for a frontend, I would assume the frontend forces debugger startup in a LANG that it knows to handle. > And if you base your detection on the presence of the error alone, then > commands that take no argument may return an error, which by no means > indicates that the command does not exist. I also assumed that such error message will not start with "Undefined MI command:". > > In addition, a yes-or-no is not even what might be needed. > > Well, the IDE team at AdaCore needs that information in order to support > the variety of GDB versions out there, and I also agreed with them that > this was a sensible request. That's fine. You want a feature, and you implement it, and you are in a position to get it in. I was merely jumping on the plural in "frontend_s_" and only because I mistook that as "all", not as "possibly more than one". > > Look e.g. at the "python" advertisement in -list-features output > > ^done,features=["frozen-varobjs","pending-breakpoints","thread-info", > > "data-read-memory-bytes","breakpoint-notifications","ada-task-info","python"] > > > > It does not indicate whether it is properly installed (datadir...) nor > > whether the version of Python is compatible with the script I want to > > execute. So in practice, checking -list-features is just extra effort > > giving only a subset of the information I would need for an "ok to use" > > decision, and it's quicker and more reliable to just execute the > > command and handle errors. > > > > It's hard to imagine that this will ever cover enough of GDB features > > and questions a frontend needs to have answered. > > If we were discussing about the specific issues regarding the use of > Python in your example, I would say that this is outside the scope of > this new command. We really aren't. "python" is an obvious case, but I could also have used "thread-info" as example. Why should a frontend ask whether "-thread-info" is supported, parse output, and switch code paths between "-thread-info" and "info threads" (and hope that an announcement of "-thread-info" existence is backed by an implementation) when it could just fire "-thread-info" and handle a possible error by falling back on "info thread"? This would mean that users of well-behaved gdb builds/installation lose one roundtrip, and the frontend needs to implement three funtions (ask, either, or) instead of two (call, fallback) > If you are trying to make a general point, then can you please tell us > how you think we can improve it? If not, you are free to find it useless > and to prefer to just use your execution test. I was indeeed trying to make a general point insofar as that I think it does not help users to introduce, or strengthen, a _third_ way to describe "the state of the nation" (first being actual behaviour of the code, second the documentation, potential third the -info-gdb-mi-command output). I actually think this very thread proves the point. You said "Recently, I added new coommands for Ada exception$ catching, but forgot about -list-features" which means there _are_ builds of gdb which support the feature, but don't announce it. Anyway, to finish this: My fourth assumption was that the "[RFC]" in the subject was short-hand for "Request for comments" and that "interested parties" were invited to comment. But I got it now. Regards, Andre' ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-14 0:36 ` André Pönitz @ 2013-11-14 9:48 ` Joel Brobecker 2013-11-14 18:31 ` André Pönitz 0 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-11-14 9:48 UTC (permalink / raw) To: André Pönitz; +Cc: Tom Tromey, gdb-patches > Anyway, to finish this: My fourth assumption was that the "[RFC]" in the > subject was short-hand for "Request for comments" and that "interested > parties" were invited to comment. But I got it now. I actually welcome comments, and you were right about the meaning of the "RFC". But receiving comments does not necessarily means agreeing with all of them. We're having a discussion, and I am trying to explain why I think the approach you are suggesting is not as practical as the one I am proposing. > > The first and most obvious to me is the case where the debugger is > > run with a non-English LANG. If you base your detection on parsing > > the error msg, then i18n ruins your plan. > > For me the context was "frontends". I assume they run external tools > in an environment that's as predictable as they need. If a user > defined LANG is problematic for a frontend, I would assume the > frontend forces debugger startup in a LANG that it knows to handle. The problem with overriding the user's LANG settings, is that you are essentially turning i18n off, thus forcing the user to see all messages from the debugger in English. Many people find that unacceptable, and I would agree with them. Besides, we've done a fair amount of work to allow i18n, so it would be a shame to see that turned off by a frontend, just to because they depend on the wording of a specific error message (which may change, btw). > This would mean that users of well-behaved gdb builds/installation > lose one roundtrip, and the frontend needs to implement three funtions > (ask, either, or) instead of two (call, fallback) I agree that FEs shouldn't be in the business of verifying that the underlying debugger is correctly built or installed. That's not what the new command is about. > I was indeeed trying to make a general point insofar as that I think it > does not help users to introduce, or strengthen, a _third_ way to > describe "the state of the nation" (first being actual behaviour of the > code, second the documentation, potential third the -info-gdb-mi-command > output). OK, I see. You're objecting to the concept itself, not the command. My stance is that I have a different assessment of the situation. I hope you'll understand why I personally think your first way (behavior of the code) is not practical - you even had to quote "works" when you proposed your approach; for the second (documentation), I hope you mean "-list-features" and not the GDB manual, and I explained why I think this is not going to scale well; that's why Tom proposed the idea of this new command. Remember also that I think the current design leaves the door open for providing more information. For instance, we could let command announce features added after the command creation. > I actually think this very thread proves the point. You said > "Recently, I added new coommands for Ada exception$ catching, but > forgot about -list-features" which means there _are_ builds of gdb > which support the feature, but don't announce it. Very possible. But I am pretty sure that no FE actually uses those features, yet - not even the AdaCore FE, since I haven't announced the feature to them yet either. For those few builds, it's OK for the FE to use the fallback mechanism. -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-14 9:48 ` Joel Brobecker @ 2013-11-14 18:31 ` André Pönitz 0 siblings, 0 replies; 42+ messages in thread From: André Pönitz @ 2013-11-14 18:31 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches On Thu, Nov 14, 2013 at 01:32:53PM +0400, Joel Brobecker wrote: > > For me the context was "frontends". I assume they run external tools > > in an environment that's as predictable as they need. If a user > > defined LANG is problematic for a frontend, I would assume the > > frontend forces debugger startup in a LANG that it knows to handle. > > The problem with overriding the user's LANG settings, is that you > are essentially turning i18n off, thus forcing the user to see > all messages from the debugger in English. Many people find that > unacceptable, and I would agree with them. Besides, we've done > a fair amount of work to allow i18n, so it would be a shame to > see that turned off by a frontend, just to because they depend > on the wording of a specific error message (which may change, btw). I think it's ok to let the frontend align debugger message output with the needs of the frontend's users. This might be translated output, it might be untranslated output, it might even be something re-phrased by the frontend. Especially for frontends supporting more than one debugger backend, each with different terminology, I see some value in using backend-agnostic messaging. It's nice that GDB provides translations, it's also nice that it gives the option to be used untranslated. [When I think about it, it's not even either-or. "Message based feature discovery" could use an second untranslated debugger instance, while the actual debugging runs in a trancelated instance...] > > This would mean that users of well-behaved gdb builds/installation > > lose one roundtrip, and the frontend needs to implement three funtions > > (ask, either, or) instead of two (call, fallback) > > I agree that FEs shouldn't be in the business of verifying that > the underlying debugger is correctly built or installed. That's > not what the new command is about. Unless the frontend bundles a "verified" build of the debugger it has to cope with what's installed in the system. That needs feature discovery, one way or the other, including recognition of what it considers "bad" installation, "bad" from the frontend's point of view, not necessarily from the distribution's point of view. > > I was indeeed trying to make a general point insofar as that I think it > > does not help users to introduce, or strengthen, a _third_ way to > > describe "the state of the nation" (first being actual behaviour of the > > code, second the documentation, potential third the -info-gdb-mi-command > > output). > > OK, I see. You're objecting to the concept itself, not the command. > My stance is that I have a different assessment of the situation. > I hope you'll understand why I personally think your first way > (behavior of the code) is not practical - you even had to quote > "works" when you proposed your approach; for the second (documentation), > I hope you mean "-list-features" and not the GDB manual, I did mean the manual. It's now certainly much better aligned to the implementation, especially for the MI bits, than it was at the 6.x times, but there are still discrepancies every now and then. Btw, one thing that I feel missing there are hints about the first appearance of a feature. Even when a frontend is ready to limit the range of supported versions of GDB', e.g. to "only >= 7.2" or "only >= 7.4" it's really hard to find out whether it's "safe" to use a feature that's mentioned in the manual. > and I explained why I think this is not going to scale well; that's why > Tom proposed the idea of this new command. I think we agree that -list-features does not scale well, and that -info-gdb-mi-command is conceptual better. We do not agree on the absolute practical utility of the feature, but that's not a problem. It was ... just a comment. Andre' ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-13 2:47 ` Joel Brobecker 2013-11-14 0:36 ` André Pönitz @ 2013-11-14 19:03 ` Pedro Alves 2013-11-14 19:37 ` Pedro Alves 1 sibling, 1 reply; 42+ messages in thread From: Pedro Alves @ 2013-11-14 19:03 UTC (permalink / raw) To: Joel Brobecker; +Cc: André Pönitz, Tom Tromey, gdb-patches On 11/13/2013 02:15 AM, Joel Brobecker wrote: >> I am not sure I agree with the judgement of benefits here. The basic >> > yes/no information is already there: >> > >> > (gdb) -unsupported-command >> > ^error,msg="Undefined MI command: unsupported-command" >> > (gdb) -symbol-list-lines >> > ^error,msg="-symbol-list-lines: Usage: SOURCE_FILENAME" >> > >> > It's not nice, but "works". > I disagree with your assessment of "works". I can think of a number > of scenarios where this would be problematic: > > The first and most obvious to me is the case where the debugger is > run with a non-English LANG. If you base your detection on parsing > the error msg, then i18n ruins your plan. And if you base your detection > on the presence of the error alone, then commands that take no argument > may return an error, which by no means indicates that the command does not > exist. Yeah. I think that points out that errors like "Undefined MI command:" and "Usage:" errors are in a different class of errors from errors caused by user input though. The former should never ever be seen by the user. They're "internal" gdb<->frontend errors. We could/should tag these differently somehow, so that the frontend doesn't have to parse a free form string. Like: "^error,msg="..." "^error,msg="...",code="unknown-command" "^error,msg="...",code="usage" or some such. This does not invalidate listing features in -list-features, as it's often useful to know upfront whether some feature is supported, so the frontend can disable parts of the GUI that won't make sense for the current target/session. -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-14 19:03 ` Pedro Alves @ 2013-11-14 19:37 ` Pedro Alves 2013-11-14 20:30 ` Tom Tromey 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2013-11-14 19:37 UTC (permalink / raw) To: Joel Brobecker; +Cc: André Pönitz, Tom Tromey, gdb-patches On 11/14/2013 06:44 PM, Pedro Alves wrote: > Yeah. I think that points out that errors like "Undefined MI command:" and > "Usage:" errors are in a different class of errors from errors caused > by user input though. The former should never ever be seen by the user. > They're "internal" gdb<->frontend errors. We could/should tag these > differently somehow, so that the frontend doesn't have to parse a > free form string. Like: > > "^error,msg="..." > "^error,msg="...",code="unknown-command" > "^error,msg="...",code="usage" > > or some such. > > This does not invalidate listing features in -list-features, as > it's often useful to know upfront whether some feature is supported, > so the frontend can disable parts of the GUI that won't make sense > for the current target/session. > Something like this: -whatever ^error,msg="Undefined MI command: whatever",error="unknown-command" (gdb) -list-thread-groups --frame 1 --frame 1 ^error,msg="Duplicate '--frame' option",error="command-usage" (gdb) Frontends must ignore unknown attributes, so it's backwards compatible. Just a POC. Of course, we'd have to go audit all MI "error" calls. --- gdb/exceptions.h | 6 ++++++ gdb/mi/mi-main.c | 13 ++++++++++++- gdb/mi/mi-parse.c | 20 +++++++++++++------- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/gdb/exceptions.h b/gdb/exceptions.h index 129d29a..07599ae 100644 --- a/gdb/exceptions.h +++ b/gdb/exceptions.h @@ -93,6 +93,12 @@ enum errors { aborted as the inferior state is no longer valid. */ TARGET_CLOSE_ERROR, + /* An unknown command was executed. */ + UNKNOWN_COMMAND_ERROR, + + /* Wrong command usage. */ + COMMAND_USAGE_ERROR, + /* Add more errors here. */ NR_ERRORS }; diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index bbf944a..c2a3988 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -2021,7 +2021,18 @@ 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 UNKNOWN_COMMAND_ERROR: + fputs_unfiltered (",error=\"unknown-command\"", raw_stdout); + break; + case COMMAND_USAGE_ERROR: + fputs_unfiltered (",error=\"command-usage\"", raw_stdout); + break; + } + fputs_unfiltered ("\n", raw_stdout); } void diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c index 9994307..2c9d267 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 (UNKNOWN_COMMAND_ERROR, + _("Undefined MI command: %s"), parse->command); /* Skip white space following the command. */ chp = skip_spaces_const (chp); @@ -324,7 +325,8 @@ mi_parse (const char *cmd, char **token) option = "--thread-group"; if (parse->thread_group != -1) - error (_("Duplicate '--thread-group' option")); + throw_error (COMMAND_USAGE_ERROR, + _("Duplicate '--thread-group' option")); chp += tgs; if (*chp != 'i') error (_("Invalid thread group id")); @@ -338,7 +340,8 @@ mi_parse (const char *cmd, char **token) option = "--thread"; if (parse->thread != -1) - error (_("Duplicate '--thread' option")); + throw_error (COMMAND_USAGE_ERROR, + _("Duplicate '--thread' option")); chp += ts; parse->thread = strtol (chp, &endp, 10); chp = endp; @@ -349,7 +352,8 @@ mi_parse (const char *cmd, char **token) option = "--frame"; if (parse->frame != -1) - error (_("Duplicate '--frame' option")); + throw_error (COMMAND_USAGE_ERROR, + _("Duplicate '--frame' option")); chp += fs; parse->frame = strtol (chp, &endp, 10); chp = endp; @@ -367,7 +371,8 @@ mi_parse (const char *cmd, char **token) parse->language = language_enum (lang_name); if (parse->language == language_unknown || parse->language == language_auto) - error (_("Invalid --language argument: %s"), lang_name); + throw_error (COMMAND_USAGE_ERROR, + _("Invalid --language argument: %s"), lang_name); do_cleanups (old_chain); } @@ -414,7 +419,8 @@ mi_parse_print_values (const char *name) || strcmp (name, mi_simple_values) == 0) return PRINT_SIMPLE_VALUES; else - error (_("Unknown value for PRINT_VALUES: must be: \ + throw_error (COMMAND_USAGE_ERROR, + _("Unknown value for PRINT_VALUES: must be: \ 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""), - mi_no_values, mi_all_values, mi_simple_values); + mi_no_values, mi_all_values, mi_simple_values); } ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-14 19:37 ` Pedro Alves @ 2013-11-14 20:30 ` Tom Tromey 2013-11-15 5:35 ` Joel Brobecker 0 siblings, 1 reply; 42+ messages in thread From: Tom Tromey @ 2013-11-14 20:30 UTC (permalink / raw) To: Pedro Alves; +Cc: Joel Brobecker, André Pönitz, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> Just a POC. Of course, we'd have to go audit all MI "error" calls. It seems like a reasonable idea to me. Tom ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-14 20:30 ` Tom Tromey @ 2013-11-15 5:35 ` Joel Brobecker 2013-11-15 12:39 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-11-15 5:35 UTC (permalink / raw) To: Tom Tromey; +Cc: Pedro Alves, André Pönitz, gdb-patches > Pedro> Just a POC. Of course, we'd have to go audit all MI "error" calls. > > It seems like a reasonable idea to me. The idea of a specific and documented error code seems much more robust to me. Regarding invalid switches, we may have to extend the current proposal to allow the command to specific what in the usage caused problem? In my proposal, it was easy to extend by adding a "feature=[...]" list to the output. Or maybe that's overkill? Or use list-features for that instead? I'd like us to decide to something I can go and implement. Either way, I think we can start by concentrating with the initial goal, which is to determine whether a command exists or not. People seem to have reacted more positively to the idea of try-and-fallback approach, shall we go with Pedro's idea (without the "invalid switch"/"usage" part)? Thanks, -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-15 5:35 ` Joel Brobecker @ 2013-11-15 12:39 ` Pedro Alves 2013-11-15 14:38 ` Joel Brobecker 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2013-11-15 12:39 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey, André Pönitz, gdb-patches On 11/15/2013 03:30 AM, Joel Brobecker wrote: >> Pedro> Just a POC. Of course, we'd have to go audit all MI "error" calls. >> >> It seems like a reasonable idea to me. > > The idea of a specific and documented error code seems much more robust > to me. > > Regarding invalid switches, we may have to extend the current proposal > to allow the command to specific what in the usage caused problem? Not sure about that. Sounds more complicated than it's worth it. > In my proposal, it was easy to extend by adding a "feature=[...]" > list to the output. Or maybe that's overkill? Or use list-features > for that instead? As list-features already exists, and works just as well, that might indeed be overkill. Or put another way, is there a use case that list-features doesn't cover, or something about "feature=[]" that'd make ours and frontend writers' lives easier? Just like with list-features, we'd always have to manually take care of listing the new command feature in "features=[]", so on our end it doesn't seem to buy anything. IOW, thinking in terms of KISS seems to suggest sticking with list-features. > I'd like us to decide to something I can go and implement. Either way, > I think we can start by concentrating with the initial goal, which is > to determine whether a command exists or not. Yeah. I have no problem with your proposal. There's actually one case where it works, and '^error,code="unknown-command"' does not, which is when a command works and has effects without options. In such cases, you can't probe for the command's existence without causing the (side) effects. > People seem to have reacted > more positively to the idea of try-and-fallback approach, shall we go > with Pedro's idea (without the "invalid switch"/"usage" part)? If I had infinite time, I'd go for all of the above. Command to probe existence of commands, and make ^error indicate both unknown command, and bad usage. :-) -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-15 12:39 ` Pedro Alves @ 2013-11-15 14:38 ` Joel Brobecker 2013-11-15 14:40 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-11-15 14:38 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, André Pönitz, gdb-patches > > Regarding invalid switches, we may have to extend the current proposal > > to allow the command to specific what in the usage caused problem? > > Not sure about that. Sounds more complicated than it's worth it. > > > In my proposal, it was easy to extend by adding a "feature=[...]" > > list to the output. Or maybe that's overkill? Or use list-features > > for that instead? > > As list-features already exists, and works just as well, that might > indeed be overkill. Or put another way, is there a use case that > list-features doesn't cover, or something about "feature=[]" > that'd make ours and frontend writers' lives easier? Just like with > list-features, we'd always have to manually take care of listing the > new command feature in "features=[]", so on our end it doesn't seem > to buy anything. > IOW, thinking in terms of KISS seems to suggest sticking with > list-features. OK, I think will work well enough in practice, and, really, worrying about a few more bytes at a time was a bit of an overreaction :). > > I'd like us to decide to something I can go and implement. Either way, > > I think we can start by concentrating with the initial goal, which is > > to determine whether a command exists or not. > > Yeah. I have no problem with your proposal. There's actually one > case where it works, and '^error,code="unknown-command"' does not, > which is when a command works and has effects without options. In such > cases, you can't probe for the command's existence without causing > the (side) effects. I think the intent was not to provide a probing mechanism, but rather to provide an approach where the FE just fires the command when it needs to, and then fallback on a CLI-based approach if detecting an 'unknown-command' error. But, on the other hand, I am thinking that some FEs might still want to probe ahead of time, for instance if they do not wish to provide a fallback mechanism (thus disabling the relevant parts of the GUI ahead of time); or even if it is easier programatically for them to probe, instead of having to handle this specific error. > > People seem to have reacted > > more positively to the idea of try-and-fallback approach, shall we go > > with Pedro's idea (without the "invalid switch"/"usage" part)? > > If I had infinite time, I'd go for all of the above. Command to > probe existence of commands, and make ^error indicate both > unknown command, and bad usage. :-) I don't have infinite amount of time, but the first 2 (new GDB/MI command and new ^error for unknown commands) are fairly small tasks, so I'm happy sending patches for both. That way, we get the best of both worlds, without must cost, I think, in terms of extra maintenance, since both patches would be pretty small, and localized. For invalid usage, I could add that to my list, but that'll have to be next year... (/me wishes I would say that on Dec 31st...) -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC] New GDB/MI command "-info-gdb-mi-command" 2013-11-15 14:38 ` Joel Brobecker @ 2013-11-15 14:40 ` Pedro Alves 2013-11-18 17:12 ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2013-11-15 14:40 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey, André Pönitz, gdb-patches On 11/15/2013 12:39 PM, Joel Brobecker wrote: >> Yeah. I have no problem with your proposal. There's actually one >> case where it works, and '^error,code="unknown-command"' does not, >> which is when a command works and has effects without options. In such >> cases, you can't probe for the command's existence without causing >> the (side) effects. > > I think the intent was not to provide a probing mechanism, but > rather to provide an approach where the FE just fires the command > when it needs to, and then fallback on a CLI-based approach if > detecting an 'unknown-command' error. Yeah. Just thinking about how we'd cover all bases if we took only one approach. > But, on the other hand, I am thinking that some FEs might still > want to probe ahead of time, for instance if they do not wish to > provide a fallback mechanism (thus disabling the relevant parts > of the GUI ahead of time); Right, that's the reasoning I usually throw around too. It's the same reasoning we probe things in the RSP upfront with qSupported. I now notice that the -list-features docu doesn't talk about that explicitly, but it could be nice to suggest it. > or even if it is easier programatically > for them to probe, instead of having to handle this specific error. >>> People seem to have reacted >>> more positively to the idea of try-and-fallback approach, shall we go >>> with Pedro's idea (without the "invalid switch"/"usage" part)? >> >> If I had infinite time, I'd go for all of the above. Command to >> probe existence of commands, and make ^error indicate both >> unknown command, and bad usage. :-) > > I don't have infinite amount of time, but the first 2 (new GDB/MI > command and new ^error for unknown commands) are fairly small tasks, > so I'm happy sending patches for both. That way, we get the best > of both worlds, without must cost, I think, in terms of extra > maintenance, since both patches would be pretty small, and localized. That sounds good to me. > For invalid usage, I could add that to my list, but that'll have > to be next year... (/me wishes I would say that on Dec 31st...) :-) -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFA GDB/MI] Help determine if GDB/MI command exists or not 2013-11-15 14:40 ` Pedro Alves @ 2013-11-18 17:12 ` Joel Brobecker 2013-11-18 17:13 ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker ` (2 more replies) 0 siblings, 3 replies; 42+ messages in thread From: Joel Brobecker @ 2013-11-18 17:12 UTC (permalink / raw) To: gdb-patches Hello, Re: http://www.sourceware.org/ml/gdb-patches/2013-11/msg00382.html This series introduces 2 patches meant to help front-ends determine whether a given GDB/MI command exists or not. - Patch #1 introduces a new command "-info-gdb-mi-command" The patch is very close to what was proposed in the original RFC, with only minor corrections, based on feedback received then; It's basically document changes, IIRC. - Path #2 implements Pedro's idea of adding an error code to the "^error" result record. I took Pedro's patch nearly verbatim, removing the bits that dealt with invalid command-line usage (this part is left for another time, if the need becomes a little more explicit). I did notice that the additonal variable looked an awful lot like an error code, so I found it odd that we'd name if "error=" in the patch. And then I realized that Pedro's first email did say "code", so I assumed it was a brain fart, and fixed the patch to use "code". Additional remarks on that patch inside the patch's revision log (to be part of the commit when eventually pushed). I added NEWS, doco, and testcase as well. Thank you, -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-11-18 17:12 ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker @ 2013-11-18 17:13 ` Joel Brobecker 2013-11-18 17:29 ` Eli Zaretskii 2013-11-18 17:21 ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker 2013-11-19 15:05 ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Pedro Alves 2 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-11-18 17:13 UTC (permalink / raw) To: gdb-patches Hello, This is mostly a re-based version of: [RFC] New GDB/MI command "-info-gdb-mi-command" http://www.sourceware.org/ml/gdb-patches/2013-11/msg00311.html I made the documentation fixes mentioned by Eli, although I wonder if the corrections were done well enough: - Added the @cindex entry for the new command; - Fixed the markup for the command's argument. I think it's worth a second review - I am not sure I did enough. Regarding some questions Eli had: | > +(the leading dash (@code{-}) in the command name should be omitted). | Is this wise? How about if we support both with and without the dash? 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) Also, looking at the grammar, the leading dash isn't listed as part of what they call the "operation" mi-command ==> [ token ] "-" operation ( " " option )* [ " --" ] ( " " parameter )* nl The other question was: | > +-info-gdb-mi-command symbol-list-lines | > +^done,command=@{exists="true"@} | > +@end smallexample | | Btw, why "command="? Perhaps "result="? I only have a mild opinion. But the choice I made seems consistent with other commands. Eg: + -list-features => features= + -thread-info => threads= + -list-thread-groups => groups= ~~~ Actual Commit's Revision Log ~~~ 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. Tested on x86_64-linux. OK to commit? Thank you, -- Joel --- gdb/NEWS | 3 +++ gdb/doc/gdb.texinfo | 47 +++++++++++++++++++++++++++++++++++++++ gdb/mi/mi-cmd-info.c | 21 +++++++++++++++++ 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 | 37 ++++++++++++++++++++++++++++++ 7 files changed, 111 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..e85b5b6 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -35069,6 +35069,51 @@ 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} +(the leading dash (@code{-}) in the command name should be omitted). + +@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 + +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 +35167,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..18f4927 100644 --- a/gdb/mi/mi-cmd-info.c +++ b/gdb/mi/mi-cmd-info.c @@ -71,6 +71,27 @@ 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]; + 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 496a8aa..aed62b2 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 83d524a..e4251c9 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..5285d31 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp @@ -0,0 +1,37 @@ +# 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 -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" + +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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-11-18 17:13 ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker @ 2013-11-18 17:29 ` Eli Zaretskii 2013-11-19 4:35 ` Joel Brobecker 0 siblings, 1 reply; 42+ messages in thread From: Eli Zaretskii @ 2013-11-18 17:29 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > From: Joel Brobecker <brobecker@adacore.com> > Date: Mon, 18 Nov 2013 21:11:58 +0400 > > Regarding some questions Eli had: > > | > +(the leading dash (@code{-}) in the command name should be omitted). > | Is this wise? How about if we support both with and without the dash? > > 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. > --- 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. > + OK for this part. > +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 > + > +And here is an example where the @sc{gdb/mi} command is known > +to the debugger: You want @noindent before "And here". The documentation parts are OK with that change. Thanks. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-11-18 17:29 ` Eli Zaretskii @ 2013-11-19 4:35 ` Joel Brobecker 2013-11-19 16:11 ` Eli Zaretskii ` (2 more replies) 0 siblings, 3 replies; 42+ messages in thread From: Joel Brobecker @ 2013-11-19 4:35 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 2278 bytes --] > > 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 [-- Attachment #2: 0001-New-GDB-MI-command-info-gdb-mi-command.patch --] [-- Type: text/x-diff, Size: 10540 bytes --] From 5a8ff8bf85f7d455c3f75312e30ec5c1e077ae01 Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> 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 <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 -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 [-- Attachment #3: info-gdb-mi-command-updates.diff --] [-- Type: text/x-diff, Size: 2833 bytes --] 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" ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-11-19 4:35 ` Joel Brobecker @ 2013-11-19 16:11 ` Eli Zaretskii 2013-12-02 3:26 ` Joel Brobecker 2013-12-02 14:53 ` Pedro Alves 2 siblings, 0 replies; 42+ messages in thread From: Eli Zaretskii @ 2013-11-19 16:11 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > Date: Tue, 19 Nov 2013 08:10:22 +0400 > From: Joel Brobecker <brobecker@adacore.com> > Cc: gdb-patches@sourceware.org > > 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. Thank you. The documentation part is still OK. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-11-19 4:35 ` Joel Brobecker 2013-11-19 16:11 ` Eli Zaretskii @ 2013-12-02 3:26 ` Joel Brobecker 2013-12-02 3:51 ` Eli Zaretskii 2013-12-02 14:53 ` Pedro Alves 2 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-12-02 3:26 UTC (permalink / raw) To: gdb-patches 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 <brobecker@adacore.com> > 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 <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 -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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-12-02 3:26 ` Joel Brobecker @ 2013-12-02 3:51 ` Eli Zaretskii 2013-12-02 4:41 ` Joel Brobecker 0 siblings, 1 reply; 42+ messages in thread From: Eli Zaretskii @ 2013-12-02 3:51 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > Date: Mon, 2 Dec 2013 07:26:15 +0400 > From: Joel Brobecker <brobecker@adacore.com> > > 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! I already approved the documentation parts, didn't I? If not, you hereby have my approval. Thanks. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-12-02 3:51 ` Eli Zaretskii @ 2013-12-02 4:41 ` Joel Brobecker 0 siblings, 0 replies; 42+ messages in thread From: Joel Brobecker @ 2013-12-02 4:41 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches > I already approved the documentation parts, didn't I? If not, you > hereby have my approval. Yes, you did, thank you! I should have been more explicit here. I'm waiting for code review. Cheers :) -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-11-19 4:35 ` Joel Brobecker 2013-11-19 16:11 ` Eli Zaretskii 2013-12-02 3:26 ` Joel Brobecker @ 2013-12-02 14:53 ` Pedro Alves 2013-12-03 4:06 ` pushed: " Joel Brobecker 2 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2013-12-02 14:53 UTC (permalink / raw) To: Joel Brobecker; +Cc: Eli Zaretskii, gdb-patches On 11/19/2013 04:10 AM, Joel Brobecker wrote: > 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? > Looks fine to me. (I don't have a strong opinion about the dash issue.) > +# 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\"" Nit, I'd suggest: "-list-features includes \"info-gdb-mi-command\"" -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* pushed: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" 2013-12-02 14:53 ` Pedro Alves @ 2013-12-03 4:06 ` Joel Brobecker 0 siblings, 0 replies; 42+ messages in thread From: Joel Brobecker @ 2013-12-03 4:06 UTC (permalink / raw) To: gdb-patches > > 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? > > > > Looks fine to me. > > (I don't have a strong opinion about the dash issue.) > > > +# 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\"" > > Nit, I'd suggest: > > "-list-features includes \"info-gdb-mi-command\"" > Thanks, Pedro. The suggestion does seem better to me too, so I applied it. Patch now in. -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFA 2/2] Add "undefined-command" error code at end of ^error result... 2013-11-18 17:12 ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker 2013-11-18 17:13 ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker @ 2013-11-18 17:21 ` Joel Brobecker 2013-11-18 17:29 ` Eli Zaretskii 2013-11-19 11:19 ` Pedro Alves 2013-11-19 15:05 ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Pedro Alves 2 siblings, 2 replies; 42+ messages in thread From: Joel Brobecker @ 2013-11-18 17:21 UTC (permalink / raw) To: gdb-patches ... 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) <UNKNOWN_COMMAND_ERROR>: New enum. * mi/mi-parse.c (mi_parse): Thow UNKNOWN_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 UNKNOWN_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. Tested on x86_64-linux. OK to commit? (hmmm, now that I have spent all that time typing everything up, I am wondering if I should rename UNKNOWN_COMMAND_ERROR into UNDEFINED_COMMAND_ERROR - no real biggie either way...) Thanks! -- Joel --- gdb/NEWS | 4 ++++ gdb/doc/gdb.texinfo | 17 ++++++++++++++-- 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, 68 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp diff --git a/gdb/NEWS b/gdb/NEWS index e61c79f..d22c56a 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -156,6 +156,10 @@ 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. + ** 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 e85b5b6..f0662ff 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -29315,11 +29315,20 @@ 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 +The operation failed. The @var{msg} variable contains the corresponding error message. +If present, the @var{code} 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 @value{GDBN} has terminated. @@ -35169,6 +35178,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..d9be897 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 unknown command was executed. */ + UNKNOWN_COMMAND_ERROR, + /* Add more errors here. */ NR_ERRORS }; diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index e4251c9..53075af 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 UNKNOWN_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 9994307..3bd7400 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 (UNKNOWN_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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result... 2013-11-18 17:21 ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker @ 2013-11-18 17:29 ` Eli Zaretskii 2013-11-19 6:02 ` Joel Brobecker 2013-11-19 11:19 ` Pedro Alves 1 sibling, 1 reply; 42+ messages in thread From: Eli Zaretskii @ 2013-11-18 17:29 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > From: Joel Brobecker <brobecker@adacore.com> > Date: Mon, 18 Nov 2013 21:11:59 +0400 > > 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." Doesn't this constitute a reason for bumping the MI revision? > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -156,6 +156,10 @@ 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. OK, but I would mention the fact that this can be inquired about, as you described in your message. > +@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. Otherwise, the documentation parts are OK. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result... 2013-11-18 17:29 ` Eli Zaretskii @ 2013-11-19 6:02 ` Joel Brobecker 2013-11-19 16:16 ` Eli Zaretskii 0 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-11-19 6:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 2802 bytes --] 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 [-- Attachment #2: 0002-Add-undefined-command-error-code-at-end-of-error-res.patch --] [-- Type: text/x-diff, Size: 8879 bytes --] 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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result... 2013-11-19 6:02 ` Joel Brobecker @ 2013-11-19 16:16 ` Eli Zaretskii 0 siblings, 0 replies; 42+ messages in thread From: Eli Zaretskii @ 2013-11-19 16:16 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > Date: Tue, 19 Nov 2013 08:35:26 +0400 > From: Joel Brobecker <brobecker@adacore.com> > Cc: gdb-patches@sourceware.org > > > > + ** 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. Thanks. > > > +@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). It's OK now. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result... 2013-11-18 17:21 ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker 2013-11-18 17:29 ` Eli Zaretskii @ 2013-11-19 11:19 ` Pedro Alves 2013-11-20 3:46 ` Joel Brobecker 1 sibling, 1 reply; 42+ messages in thread From: Pedro Alves @ 2013-11-19 11:19 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 11/18/2013 05:11 PM, Joel Brobecker wrote: > gdb/ChangeLog: > > (from Pedro Alves <palves@redhat.com>) > (from Joel Brobecker <brobecker@adacore.com>) > * exceptions.h (enum_errors) <UNKNOWN_COMMAND_ERROR>: New enum. > * mi/mi-parse.c (mi_parse): Thow UNKNOWN_COMMAND_ERROR instead "Throw" > 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 UNKNOWN_COMMAND_ERROR. s/in/is ? > * 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. > > Tested on x86_64-linux. OK to commit? Looks good to me... > (hmmm, now that I have spent all that time typing everything up, > I am wondering if I should rename UNKNOWN_COMMAND_ERROR into > UNDEFINED_COMMAND_ERROR - no real biggie either way...) Yeah, I think so. The CLI also says "undefined": (gdb) asdf Undefined command: "asdf". Try "help". Might as well be consistent throughout. -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result... 2013-11-19 11:19 ` Pedro Alves @ 2013-11-20 3:46 ` Joel Brobecker 2013-12-03 4:08 ` pushed: " Joel Brobecker 0 siblings, 1 reply; 42+ messages in thread From: Joel Brobecker @ 2013-11-20 3:46 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 1922 bytes --] On Tue, Nov 19, 2013 at 11:13:30AM +0000, Pedro Alves wrote: > On 11/18/2013 05:11 PM, Joel Brobecker wrote: > > > gdb/ChangeLog: > > > > (from Pedro Alves <palves@redhat.com>) > > (from Joel Brobecker <brobecker@adacore.com>) > > * exceptions.h (enum_errors) <UNKNOWN_COMMAND_ERROR>: New enum. > > * mi/mi-parse.c (mi_parse): Thow UNKNOWN_COMMAND_ERROR instead > > "Throw" > > > 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 UNKNOWN_COMMAND_ERROR. > > s/in/is ? > > > * 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. > > > > Tested on x86_64-linux. OK to commit? > > Looks good to me... Thanks, Pedro! I've made the corrections for the errors you spotted. And for the UNKNOWN_COMMAND_ERROR vs UNDEFINED_COMMAND_ERROR, I had already made the changes in the first re-send, for the documentation fixes. This patch depends on patch #1, not logically, but there are conflicts areas that I'd rather not have to deal with if I don't have to. I am just hoping for someone to review the patch (hint, hint! :-)). If it takes too much time, I will put this one in first. -- Joel [-- Attachment #2: 0002-Add-undefined-command-error-code-at-end-of-error-res.patch --] [-- Type: text/x-diff, Size: 8874 bytes --] From 902182d958a76a09d9a551c5121fe15e10d2d5bc 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): Throw 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 is 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..4aef805 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 on 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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* pushed: [RFA 2/2] Add "undefined-command" error code at end of ^error result... 2013-11-20 3:46 ` Joel Brobecker @ 2013-12-03 4:08 ` Joel Brobecker 0 siblings, 0 replies; 42+ messages in thread From: Joel Brobecker @ 2013-12-03 4:08 UTC (permalink / raw) To: gdb-patches > > > gdb/ChangeLog: > > > > > > (from Pedro Alves <palves@redhat.com>) > > > (from Joel Brobecker <brobecker@adacore.com>) > > > * exceptions.h (enum_errors) <UNKNOWN_COMMAND_ERROR>: New enum. > > > * mi/mi-parse.c (mi_parse): Thow UNKNOWN_COMMAND_ERROR instead > > > > "Throw" > > > > > 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 UNKNOWN_COMMAND_ERROR. > > > > s/in/is ? > > > > > * 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. > > > > > > Tested on x86_64-linux. OK to commit? Now pushed. -- Joel ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFA GDB/MI] Help determine if GDB/MI command exists or not 2013-11-18 17:12 ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker 2013-11-18 17:13 ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker 2013-11-18 17:21 ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker @ 2013-11-19 15:05 ` Pedro Alves 2 siblings, 0 replies; 42+ messages in thread From: Pedro Alves @ 2013-11-19 15:05 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 11/18/2013 05:11 PM, Joel Brobecker wrote: > - Path #2 implements Pedro's idea of adding an error code to > the "^error" result record. > > I took Pedro's patch nearly verbatim, removing the bits that > dealt with invalid command-line usage (this part is left for > another time, if the need becomes a little more explicit). > I did notice that the additonal variable looked an awful lot > like an error code, so I found it odd that we'd name if "error=" > in the patch. And then I realized that Pedro's first email did > say "code", so I assumed it was a brain fart, and fixed the patch > to use "code". Whoops, yeah. Thanks, -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2013-12-03 4:08 UTC | newest] Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-11-10 17:16 [RFC] Add ada-exception-catchpoints to -list-features command output Joel Brobecker 2013-11-10 22:16 ` Eli Zaretskii 2013-11-12 11:25 ` Joel Brobecker 2013-11-12 16:39 ` Eli Zaretskii 2013-11-13 3:02 ` Joel Brobecker 2013-11-11 15:22 ` Tom Tromey 2013-11-12 9:18 ` Joel Brobecker 2013-11-12 12:11 ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker 2013-11-12 17:04 ` Eli Zaretskii 2013-11-12 17:48 ` Joel Brobecker 2013-11-12 18:34 ` Eli Zaretskii 2013-11-13 3:19 ` Joel Brobecker 2013-11-12 21:17 ` André Pönitz 2013-11-13 2:47 ` Joel Brobecker 2013-11-14 0:36 ` André Pönitz 2013-11-14 9:48 ` Joel Brobecker 2013-11-14 18:31 ` André Pönitz 2013-11-14 19:03 ` Pedro Alves 2013-11-14 19:37 ` Pedro Alves 2013-11-14 20:30 ` Tom Tromey 2013-11-15 5:35 ` Joel Brobecker 2013-11-15 12:39 ` Pedro Alves 2013-11-15 14:38 ` Joel Brobecker 2013-11-15 14:40 ` Pedro Alves 2013-11-18 17:12 ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker 2013-11-18 17:13 ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker 2013-11-18 17:29 ` Eli Zaretskii 2013-11-19 4:35 ` Joel Brobecker 2013-11-19 16:11 ` Eli Zaretskii 2013-12-02 3:26 ` Joel Brobecker 2013-12-02 3:51 ` Eli Zaretskii 2013-12-02 4:41 ` Joel Brobecker 2013-12-02 14:53 ` Pedro Alves 2013-12-03 4:06 ` pushed: " Joel Brobecker 2013-11-18 17:21 ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker 2013-11-18 17:29 ` Eli Zaretskii 2013-11-19 6:02 ` Joel Brobecker 2013-11-19 16:16 ` Eli Zaretskii 2013-11-19 11:19 ` Pedro Alves 2013-11-20 3:46 ` Joel Brobecker 2013-12-03 4:08 ` pushed: " Joel Brobecker 2013-11-19 15:05 ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox