[RFA,06/10] Fix inconsistent output of prefix and bugs in 'show' command

Message ID 20200510205530.21923-7-philippe.waroquiers@skynet.be
State Superseded
Headers show
Series
  • fix/improve cmd structure, class_alias, help, apropos
Related show

Commit Message

H.J. Lu via Gdb-patches May 10, 2020, 8:55 p.m.
cmd_show_list function implements the 'show' command.

cmd_show_list output is inconsistent: it sometimes shows a prefix
and sometimes does not.
For example, in the below, you see that there is a prefix before
each value, except for 'enabled'.

    (gdb) show style
    style address background:  The "address" style background color is: none
    style address foreground:  The "address" style foreground color is: blue
    style address intensity:  The "address" style display intensity is: normal
    enabled:  CLI output styling is enabled.
    style filename background:  The "filename" style background color is: none
    ...

There are other inconsistencies or bugs e.g. in
the below we see twice insn-number-max, once with a prefix
and once without prefix : last line, just before the value of
instruction-history-size which is itself without prefix.

    (gdb) show record
    record btrace bts buffer-size:  The record/replay bts buffer size is 65536.
    record btrace cpu:  btrace cpu is 'auto'.
    record btrace pt buffer-size:  The record/replay pt buffer size is 16384.
    record btrace replay-memory-access:  Replay memory access is read-only.
    record full insn-number-max:  Record/replay buffer limit is 200000.
    record full memory-query:  Whether query if PREC cannot record memory change of next instruction is off.
    record full stop-at-limit:  Whether record/replay stops when record/replay buffer becomes full is on.
    function-call-history-size:  Number of functions to print in "record function-call-history" is 10.
    insn-number-max:  instruction-history-size:  Number of instructions to print in "record instruction-history" is 10.
    (gdb)

Also, some values are output several times due to some aliases, so avoid outputting duplicated
values by skipping all aliases.

Now that the command structure has a correct 'back-pointer' from a command
to its prefix command, we can simplify cmd_show_list by removing its prefix argument
and at the same time fix the output inconsistencies and bugs.

gdb/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-setshow.h (cmd_show_list): Remove prefix argument.
	* cli/cli-decode.c (do_show_prefix_cmd): Likewise.
	* command.h (cmd_show_list): Likewise.
	* dwarf2/index-cache.c (show_index_cache_command): Likewise.
	* cli/cli-setshow.c (cmd_show_list): Use the prefix to produce the output.  Skip aliases.

gdb/testsuite/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/default.exp: Update output following fixes.
---
 gdb/cli/cli-decode.c               |  2 +-
 gdb/cli/cli-setshow.c              | 38 +++++++++++++++++-------------
 gdb/cli/cli-setshow.h              |  3 +--
 gdb/command.h                      |  2 +-
 gdb/dwarf2/index-cache.c           |  2 +-
 gdb/testsuite/gdb.base/default.exp |  4 ++--
 6 files changed, 28 insertions(+), 23 deletions(-)

-- 
2.20.1

Comments

Tom Tromey May 14, 2020, 4:16 p.m. | #1
>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:


Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* cli/cli-setshow.h (cmd_show_list): Remove prefix argument.
Philippe> 	* cli/cli-decode.c (do_show_prefix_cmd): Likewise.
Philippe> 	* command.h (cmd_show_list): Likewise.
Philippe> 	* dwarf2/index-cache.c (show_index_cache_command): Likewise.
Philippe> 	* cli/cli-setshow.c (cmd_show_list): Use the prefix to produce the output.  Skip aliases.

Looks good.  Thank you.

Philippe> -      if (list->prefixlist && !list->abbrev_flag)
Philippe> +      if (list->prefixlist && list->cmd_pointer == nullptr)

It wasn't immediately apparent to me why checking cmd_pointer is the
correct thing to do here, but that's just because I forgot what it was
for.  The comment in cli-decode.h made it clear.

Tom

Patch

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index be36eb6732..c37dfaffb5 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -428,7 +428,7 @@  add_basic_prefix_cmd (const char *name, enum command_class theclass,
 static void
 do_show_prefix_cmd (const char *args, int from_tty, struct cmd_list_element *c)
 {
-  cmd_show_list (*c->prefixlist, from_tty, "");
+  cmd_show_list (*c->prefixlist, from_tty);
 }
 
 /* See command.h.  */
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 2a2f905bdf..19a5565bfe 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -733,39 +733,45 @@  do_show_command (const char *arg, int from_tty, struct cmd_list_element *c)
 /* Show all the settings in a list of show commands.  */
 
 void
-cmd_show_list (struct cmd_list_element *list, int from_tty, const char *prefix)
+cmd_show_list (struct cmd_list_element *list, int from_tty)
 {
   struct ui_out *uiout = current_uiout;
 
   ui_out_emit_tuple tuple_emitter (uiout, "showlist");
   for (; list != NULL; list = list->next)
     {
+      /* We skip show command aliases to avoid showing duplicated values.  */
+
       /* If we find a prefix, run its list, prefixing our output by its
          prefix (with "show " skipped).  */
-      if (list->prefixlist && !list->abbrev_flag)
+      if (list->prefixlist && list->cmd_pointer == nullptr)
 	{
 	  ui_out_emit_tuple optionlist_emitter (uiout, "optionlist");
 	  const char *new_prefix = strstr (list->prefixname, "show ") + 5;
 
 	  if (uiout->is_mi_like_p ())
 	    uiout->field_string ("prefix", new_prefix);
-	  cmd_show_list (*list->prefixlist, from_tty, new_prefix);
+	  cmd_show_list (*list->prefixlist, from_tty);
 	}
-      else
+      else if (list->theclass != no_set_class && list->cmd_pointer == nullptr)
 	{
-	  if (list->theclass != no_set_class)
-	    {
-	      ui_out_emit_tuple option_emitter (uiout, "option");
-
-	      uiout->text (prefix);
-	      uiout->field_string ("name", list->name);
-	      uiout->text (":  ");
-	      if (list->type == show_cmd)
-		do_show_command (NULL, from_tty, list);
-	      else
-		cmd_func (list, NULL, from_tty);
-	    }
+	  ui_out_emit_tuple option_emitter (uiout, "option");
+
+	  {
+	    /* If we find a prefix, output it (with "show " skipped).  */
+	    const char *prefixname
+	      = (list->prefix == nullptr ? ""
+		 : strstr (list->prefix->prefixname, "show ") + 5);
+	    uiout->text (prefixname);
+	  }
+	  uiout->field_string ("name", list->name);
+	  uiout->text (":  ");
+	  if (list->type == show_cmd)
+	    do_show_command (NULL, from_tty, list);
+	  else
+	    cmd_func (list, NULL, from_tty);
 	}
     }
 }
 
+
diff --git a/gdb/cli/cli-setshow.h b/gdb/cli/cli-setshow.h
index 9f0492bd32..83e4984ed6 100644
--- a/gdb/cli/cli-setshow.h
+++ b/gdb/cli/cli-setshow.h
@@ -60,7 +60,6 @@  extern void do_show_command (const char *arg, int from_tty,
 /* Get a string version of C's current value.  */
 extern std::string get_setshow_command_value_string (const cmd_list_element *c);
 
-extern void cmd_show_list (struct cmd_list_element *list, int from_tty,
-			   const char *prefix);
+extern void cmd_show_list (struct cmd_list_element *list, int from_tty);
 
 #endif /* CLI_CLI_SETSHOW_H */
diff --git a/gdb/command.h b/gdb/command.h
index 81c35c98d2..0a1706c545 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -464,7 +464,7 @@  extern void
 
 /* Do a "show" command for each thing on a command list.  */
 
-extern void cmd_show_list (struct cmd_list_element *, int, const char *);
+extern void cmd_show_list (struct cmd_list_element *, int);
 
 /* Used everywhere whenever at least one parameter is required and
    none is specified.  */
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index 23e938b010..76e7ce6ab7 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -259,7 +259,7 @@  show_index_cache_command (const char *arg, int from_tty)
   auto restore_flag = make_scoped_restore (&in_show_index_cache_command, true);
 
   /* Call all "show index-cache" subcommands.  */
-  cmd_show_list (show_index_cache_prefix_list, from_tty, "");
+  cmd_show_list (show_index_cache_prefix_list, from_tty);
 
   printf_unfiltered ("\n");
   printf_unfiltered
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 95b92c4871..c34bb9a92a 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -559,7 +559,7 @@  gdb_test "show args" "Argument list to give program being debugged when it is st
 
 # test show check abbreviations
 foreach x {"c" "ch" "check"} {
-    gdb_test "show $x" "range:  *Range checking is \"auto; currently off\".(\[^\r\n\]*\[\r\n\])+type:  *Strict type checking is on\..*" \
+    gdb_test "show $x" "check range:  *Range checking is \"auto; currently off\".(\[^\r\n\]*\[\r\n\])+check type:  *Strict type checking is on\..*" \
 	"show check \"$x\" abbreviation"
 }
 
@@ -645,7 +645,7 @@  gdb_test "show history save" "Saving of the history record on exit is on."
 #test show history size
 gdb_test "show history size" "The size of the command history is.*"
 #test show history
-gdb_test "show history" "expansion:  *History expansion on command input is o(\[^\r\n\]*\[\r\n\])+filename:  *The filename in which to record the command history is.*.gdb_history(\[^\r\n\]*\[\r\n\])+save: *Saving of the history record on exit is o(\[^\r\n\]*\[\r\n\])+size: * The size of the command history is.*"
+gdb_test "show history" "history expansion:  *History expansion on command input is o(\[^\r\n\]*\[\r\n\])+history filename:  *The filename in which to record the command history is.*.gdb_history(\[^\r\n\]*\[\r\n\])+history save: *Saving of the history record on exit is o(\[^\r\n\]*\[\r\n\])+history size: * The size of the command history is.*"
 #test show language
 gdb_test "show language" "The current source language is \"auto; currently c\"."
 #test show listsize