public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/4] layout command changes
@ 2015-05-20 23:17 Andrew Burgess
  2015-05-20 23:18 ` [PATCH 2/4] gdb: Add completer for layout command Andrew Burgess
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Andrew Burgess @ 2015-05-20 23:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

This patch set replaces an earlier patch I posted here:
  https://sourceware.org/ml/gdb-patches/2015-04/msg00185.html

In the previous patch I had to jump through some hoops in order to
support completion of the layout names like $FREGS.  This was pretty
annoying as I had not realised these layouts existed until I started
writting the completer code...

...but it turns out that those layout names don't work anyway, and
have not done so for some time.  I didn't figure out exactly when they
broke, but I believe they were broken in 6.8.

Still, it doesn't matter, as we have the 'tui regs' command, which
does work, and does allow the register set displayed in tui to be
changed.  This is for the best anyway (I think), personally, I felt
that managing both the layout, and the choice of register set all from
the layout command was too much overloading.

The first patch in this series removes the $FREGS style register set
names from the layout command, and cleans up all of the code relating
to them.

The second patch is a much simpler version of command completion
support for layout names.

The third and forth patches fix small tui related issues that I
spotted during testing.

Thanks,
Andrew

--

Andrew Burgess (4):
  gdb: Remove register class specific layout names.
  gdb: Add completer for layout command.
  gdb: Don't call tui_enable too early.
  gdb: Add cleanup to avoid memory leak on error.

 gdb/ChangeLog                         |  49 ++++++
 gdb/testsuite/ChangeLog               |   5 +
 gdb/testsuite/gdb.base/completion.exp |  19 +++
 gdb/tui/tui-data.c                    |  10 +-
 gdb/tui/tui-data.h                    |  23 ---
 gdb/tui/tui-layout.c                  | 296 +++++++++++++++-------------------
 gdb/tui/tui-layout.h                  |   3 +-
 gdb/tui/tui.c                         |   8 +-
 8 files changed, 210 insertions(+), 203 deletions(-)

-- 
2.4.0

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

* [PATCH 1/4] gdb: Remove register class specific layout names.
  2015-05-20 23:17 [PATCH 0/4] layout command changes Andrew Burgess
                   ` (2 preceding siblings ...)
  2015-05-20 23:18 ` [PATCH 3/4] gdb: Don't call tui_enable too early Andrew Burgess
@ 2015-05-20 23:18 ` Andrew Burgess
  2015-05-21  8:42   ` Pedro Alves
  2015-05-21 12:25   ` Andrew Burgess
  2015-05-21  8:12 ` [PATCH 0/4] layout command changes Pedro Alves
  4 siblings, 2 replies; 14+ messages in thread
From: Andrew Burgess @ 2015-05-20 23:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

The layout command supports the layout names $FREGS, $GREGS, $SREGS,
and $REGS. The intention of these layout names was to display the tui
register window with a specific set of registers.

First, these layout names no longer work, and haven't for a while, using
any of them will just result in switching to the general register view.

Second there is already the command 'tui reg GROUP' command to set the
displayed register set to GROUP, so making the layout command also
control the register set feels like unnecessary overloading of the
layout command.

This commit removes all code relating to supporting the register set
specific names from the layout command.  Afterwards the user can select
an available layout using the layout command, and control the choice of
register set using the 'tui reg GROUP' command.

gdb/ChangeLog:

	* tui/tui-layout.c (tui_set_layout): Remove
	tui_register_display_type parameter.  Remove all checking of this
	parameter, and reindent function.  Update header comment.
	(tui_set_layout_for_display_command): Don't check for different
	register class types, don't pass a tui_register_display_type to
	tui_set_layout.
	(layout_names): Remove register set specific names.
	* tui/tui-layout.h (tui_set_layout): Remove
	tui_register_display_type parameter.
	* tui/tui.c (tui_rl_change_windows): Don't pass a
	tui_register_display_type to tui_set_layout.
	(tui_rl_delete_other_windows): Likewise.
	(tui_enable): Likewise.
	* tui/tui-data.h (TUI_FLOAT_REGS_NAME): Remove.
	(TUI_FLOAT_REGS_NAME_LOWER): Remove.
	(TUI_GENERAL_REGS_NAME): Remove.
	(TUI_GENERAL_REGS_NAME_LOWER): Remove.
	(TUI_SPECIAL_REGS_NAME): Remove.
	(TUI_SPECIAL_REGS_NAME_LOWER): Remove.
	(TUI_GENERAL_SPECIAL_REGS_NAME): Remove.
	(TUI_GENERAL_SPECIAL_REGS_NAME_LOWER): Remove.
	(enum tui_register_display_type): Remove.
	(struct tui_layout_def): Remove regs_display_type and
	float_regs_display_type fields.
	(struct tui_data_info): Remove regs_display_type field.
	* tui/tui-data.c (layout_def): Don't initialise removed fields.
	(tui_clear_win_detail): Don't initialise removed fields of
	win_info.
---
 gdb/ChangeLog        |  31 +++++++
 gdb/tui/tui-data.c   |  10 +-
 gdb/tui/tui-data.h   |  23 -----
 gdb/tui/tui-layout.c | 251 +++++++++++++++++++--------------------------------
 gdb/tui/tui-layout.h |   3 +-
 gdb/tui/tui.c        |   8 +-
 6 files changed, 127 insertions(+), 199 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 28d6b71..40c70e7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,34 @@
+2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* tui/tui-layout.c (tui_set_layout): Remove
+	tui_register_display_type parameter.  Remove all checking of this
+	parameter, and reindent function.  Update header comment.
+	(tui_set_layout_for_display_command): Don't check for different
+	register class types, don't pass a tui_register_display_type to
+	tui_set_layout.
+	(layout_names): Remove register set specific names.
+	* tui/tui-layout.h (tui_set_layout): Remove
+	tui_register_display_type parameter.
+	* tui/tui.c (tui_rl_change_windows): Don't pass a
+	tui_register_display_type to tui_set_layout.
+	(tui_rl_delete_other_windows): Likewise.
+	(tui_enable): Likewise.
+	* tui/tui-data.h (TUI_FLOAT_REGS_NAME): Remove.
+	(TUI_FLOAT_REGS_NAME_LOWER): Remove.
+	(TUI_GENERAL_REGS_NAME): Remove.
+	(TUI_GENERAL_REGS_NAME_LOWER): Remove.
+	(TUI_SPECIAL_REGS_NAME): Remove.
+	(TUI_SPECIAL_REGS_NAME_LOWER): Remove.
+	(TUI_GENERAL_SPECIAL_REGS_NAME): Remove.
+	(TUI_GENERAL_SPECIAL_REGS_NAME_LOWER): Remove.
+	(enum tui_register_display_type): Remove.
+	(struct tui_layout_def): Remove regs_display_type and
+	float_regs_display_type fields.
+	(struct tui_data_info): Remove regs_display_type field.
+	* tui/tui-data.c (layout_def): Don't initialise removed fields.
+	(tui_clear_win_detail): Don't initialise removed fields of
+	win_info.
+
 2015-05-20  Joel Brobecker  <brobecker@adacore.com>
 
 	* infrun.c (handle_inferior_event_1): Renames handle_inferior_event.
diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index ffd80d5..ed42c8d 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -44,9 +44,7 @@ static int default_tab_len = DEFAULT_TAB_LEN;
 static struct tui_win_info *win_with_focus = (struct tui_win_info *) NULL;
 static struct tui_layout_def layout_def = {
   SRC_WIN,			/* DISPLAY_MODE */
-  FALSE,			/* SPLIT */
-  TUI_UNDEFINED_REGS,		/* REGS_DISPLAY_TYPE */
-  TUI_SFLOAT_REGS};		/* FLOAT_REGS_DISPLAY_TYPE */
+  FALSE};			/* SPLIT */
 
 static int win_resized = FALSE;
 
@@ -224,8 +222,6 @@ tui_clear_win_detail (struct tui_win_info *win_info)
 	  win_info->detail.data_display_info.regs_content =
 	    (tui_win_content) NULL;
 	  win_info->detail.data_display_info.regs_content_count = 0;
-	  win_info->detail.data_display_info.regs_display_type =
-	    TUI_UNDEFINED_REGS;
 	  win_info->detail.data_display_info.regs_column_count = 1;
 	  win_info->detail.data_display_info.display_regs = FALSE;
 	  break;
@@ -544,8 +540,6 @@ init_win_info (struct tui_win_info *win_info)
       win_info->detail.data_display_info.data_content_count = 0;
       win_info->detail.data_display_info.regs_content = (tui_win_content) NULL;
       win_info->detail.data_display_info.regs_content_count = 0;
-      win_info->detail.data_display_info.regs_display_type =
-	TUI_UNDEFINED_REGS;
       win_info->detail.data_display_info.regs_column_count = 1;
       win_info->detail.data_display_info.display_regs = FALSE;
       win_info->detail.data_display_info.current_group = 0;
@@ -745,8 +739,6 @@ tui_free_window (struct tui_win_info *win_info)
 	  win_info->detail.data_display_info.data_content =
 	    (tui_win_content) NULL;
 	  win_info->detail.data_display_info.data_content_count = 0;
-	  win_info->detail.data_display_info.regs_display_type =
-	    TUI_UNDEFINED_REGS;
 	  win_info->detail.data_display_info.regs_column_count = 1;
 	  win_info->detail.data_display_info.display_regs = FALSE;
 	  win_info->generic.content = NULL;
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 7651efd..05263e3 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -92,15 +92,6 @@ struct tui_gen_win_info
 #define MAX_TARGET_WIDTH  10
 #define MAX_PID_WIDTH     19
 
-#define TUI_FLOAT_REGS_NAME                  "$FREGS"
-#define TUI_FLOAT_REGS_NAME_LOWER            "$fregs"
-#define TUI_GENERAL_REGS_NAME                "$GREGS"
-#define TUI_GENERAL_REGS_NAME_LOWER          "$gregs"
-#define TUI_SPECIAL_REGS_NAME                "$SREGS"
-#define TUI_SPECIAL_REGS_NAME_LOWER          "$sregs"
-#define TUI_GENERAL_SPECIAL_REGS_NAME        "$REGS"
-#define TUI_GENERAL_SPECIAL_REGS_NAME_LOWER  "$regs"
-
 /* Scroll direction enum.  */
 enum tui_scroll_direction
 {
@@ -139,17 +130,6 @@ enum tui_data_type
   TUI_STRUCT
 };
 
-/* Types of register displays.  */
-enum tui_register_display_type
-{
-  TUI_UNDEFINED_REGS,
-  TUI_GENERAL_REGS,
-  TUI_SFLOAT_REGS,
-  TUI_DFLOAT_REGS,
-  TUI_SPECIAL_REGS,
-  TUI_GENERAL_AND_SPECIAL_REGS
-};
-
 enum tui_line_or_address_kind
 {
   LOA_LINE,
@@ -172,8 +152,6 @@ struct tui_layout_def
 {
   enum tui_win_type display_mode;
   int split;
-  enum tui_register_display_type regs_display_type;
-  enum tui_register_display_type float_regs_display_type;
 };
 
 /* Elements in the Source/Disassembly Window.  */
@@ -263,7 +241,6 @@ struct tui_data_info
   int data_content_count;
   tui_win_content regs_content;	/* Start of regs display content.  */
   int regs_content_count;
-  enum tui_register_display_type regs_display_type;
   int regs_column_count;
   int display_regs;		/* Should regs be displayed at all?  */
   struct reggroup *current_group;
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 4c25d43..44aca5d 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -122,18 +122,13 @@ show_layout (enum tui_layout_type layout)
 
 
 /* Function to set the layout to SRC_COMMAND, DISASSEM_COMMAND,
-   SRC_DISASSEM_COMMAND, SRC_DATA_COMMAND, or DISASSEM_DATA_COMMAND.
-   If the layout is SRC_DATA_COMMAND, DISASSEM_DATA_COMMAND, or
-   UNDEFINED_LAYOUT, then the data window is populated according to
-   regs_display_type.  */
+   SRC_DISASSEM_COMMAND, SRC_DATA_COMMAND, or DISASSEM_DATA_COMMAND.  */
 enum tui_status
-tui_set_layout (enum tui_layout_type layout_type,
-		enum tui_register_display_type regs_display_type)
+tui_set_layout (enum tui_layout_type layout_type)
 {
   enum tui_status status = TUI_SUCCESS;
 
-  if (layout_type != UNDEFINED_LAYOUT 
-      || regs_display_type != TUI_UNDEFINED_REGS)
+  if (layout_type != UNDEFINED_LAYOUT)
     {
       enum tui_layout_type cur_layout = tui_current_layout (),
 	new_layout = UNDEFINED_LAYOUT;
@@ -145,113 +140,98 @@ tui_set_layout (enum tui_layout_type layout_type,
 
       extract_display_start_addr (&gdbarch, &addr);
 
-      if (layout_type == UNDEFINED_LAYOUT
-	  && regs_display_type != TUI_UNDEFINED_REGS)
-	{
-	  if (cur_layout == SRC_DISASSEM_COMMAND)
-	    new_layout = DISASSEM_DATA_COMMAND;
-	  else if (cur_layout == SRC_COMMAND 
-		   || cur_layout == SRC_DATA_COMMAND)
-	    new_layout = SRC_DATA_COMMAND;
-	  else if (cur_layout == DISASSEM_COMMAND 
-		   || cur_layout == DISASSEM_DATA_COMMAND)
-	    new_layout = DISASSEM_DATA_COMMAND;
-	}
-      else
-	new_layout = layout_type;
+      new_layout = layout_type;
 
-      regs_populate = (new_layout == SRC_DATA_COMMAND 
-		       || new_layout == DISASSEM_DATA_COMMAND 
-		       || regs_display_type != TUI_UNDEFINED_REGS);
-      if (new_layout != cur_layout
-	  || regs_display_type != TUI_UNDEFINED_REGS)
+      regs_populate = (new_layout == SRC_DATA_COMMAND
+		       || new_layout == DISASSEM_DATA_COMMAND);
+      if (new_layout != cur_layout)
 	{
-	  if (new_layout != cur_layout)
-	    {
-	      show_layout (new_layout);
+	  show_layout (new_layout);
 
-	      /* Now determine where focus should be.  */
-	      if (win_with_focus != TUI_CMD_WIN)
+	  /* Now determine where focus should be.  */
+	  if (win_with_focus != TUI_CMD_WIN)
+	    {
+	      switch (new_layout)
 		{
-		  switch (new_layout)
-		    {
-		    case SRC_COMMAND:
-		      tui_set_win_focus_to (TUI_SRC_WIN);
-		      layout_def->display_mode = SRC_WIN;
-		      layout_def->split = FALSE;
-		      break;
-		    case DISASSEM_COMMAND:
-		      /* The previous layout was not showing code.
-		         This can happen if there is no source
-		         available:
-
-		         1. if the source file is in another dir OR
-		         2. if target was compiled without -g
-		         We still want to show the assembly though!  */
-
-		      tui_get_begin_asm_address (&gdbarch, &addr);
-		      tui_set_win_focus_to (TUI_DISASM_WIN);
-		      layout_def->display_mode = DISASSEM_WIN;
-		      layout_def->split = FALSE;
-		      break;
-		    case SRC_DISASSEM_COMMAND:
-		      /* The previous layout was not showing code.
-		         This can happen if there is no source
-		         available:
-
-		         1. if the source file is in another dir OR
-		         2. if target was compiled without -g
-		         We still want to show the assembly though!  */
-
-		      tui_get_begin_asm_address (&gdbarch, &addr);
-		      if (win_with_focus == TUI_SRC_WIN)
-			tui_set_win_focus_to (TUI_SRC_WIN);
-		      else
-			tui_set_win_focus_to (TUI_DISASM_WIN);
-		      layout_def->split = TRUE;
-		      break;
-		    case SRC_DATA_COMMAND:
-		      if (win_with_focus != TUI_DATA_WIN)
-			tui_set_win_focus_to (TUI_SRC_WIN);
-		      else
-			tui_set_win_focus_to (TUI_DATA_WIN);
-		      layout_def->display_mode = SRC_WIN;
-		      layout_def->split = FALSE;
-		      break;
-		    case DISASSEM_DATA_COMMAND:
-		      /* The previous layout was not showing code.
-		         This can happen if there is no source
-		         available:
-
-			 1. if the source file is in another dir OR
-		         2. if target was compiled without -g
-		         We still want to show the assembly though!  */
-
-		      tui_get_begin_asm_address (&gdbarch, &addr);
-		      if (win_with_focus != TUI_DATA_WIN)
-			tui_set_win_focus_to (TUI_DISASM_WIN);
-		      else
-			tui_set_win_focus_to (TUI_DATA_WIN);
-		      layout_def->display_mode = DISASSEM_WIN;
-		      layout_def->split = FALSE;
-		      break;
-		    default:
-		      break;
-		    }
+		case SRC_COMMAND:
+		  tui_set_win_focus_to (TUI_SRC_WIN);
+		  layout_def->display_mode = SRC_WIN;
+		  layout_def->split = FALSE;
+		  break;
+		case DISASSEM_COMMAND:
+		  /* The previous layout was not showing code.
+		     This can happen if there is no source
+		     available:
+
+		     1. if the source file is in another dir OR
+		     2. if target was compiled without -g
+		     We still want to show the assembly though!  */
+
+		  tui_get_begin_asm_address (&gdbarch, &addr);
+		  tui_set_win_focus_to (TUI_DISASM_WIN);
+		  layout_def->display_mode = DISASSEM_WIN;
+		  layout_def->split = FALSE;
+		  break;
+		case SRC_DISASSEM_COMMAND:
+		  /* The previous layout was not showing code.
+		     This can happen if there is no source
+		     available:
+
+		     1. if the source file is in another dir OR
+		     2. if target was compiled without -g
+		     We still want to show the assembly though!  */
+
+		  tui_get_begin_asm_address (&gdbarch, &addr);
+		  if (win_with_focus == TUI_SRC_WIN)
+		    tui_set_win_focus_to (TUI_SRC_WIN);
+		  else
+		    tui_set_win_focus_to (TUI_DISASM_WIN);
+		  layout_def->split = TRUE;
+		  break;
+		case SRC_DATA_COMMAND:
+		  if (win_with_focus != TUI_DATA_WIN)
+		    tui_set_win_focus_to (TUI_SRC_WIN);
+		  else
+		    tui_set_win_focus_to (TUI_DATA_WIN);
+		  layout_def->display_mode = SRC_WIN;
+		  layout_def->split = FALSE;
+		  break;
+		case DISASSEM_DATA_COMMAND:
+		  /* The previous layout was not showing code.
+		     This can happen if there is no source
+		     available:
+
+		     1. if the source file is in another dir OR
+		     2. if target was compiled without -g
+		     We still want to show the assembly though!  */
+
+		  tui_get_begin_asm_address (&gdbarch, &addr);
+		  if (win_with_focus != TUI_DATA_WIN)
+		    tui_set_win_focus_to (TUI_DISASM_WIN);
+		  else
+		    tui_set_win_focus_to (TUI_DATA_WIN);
+		  layout_def->display_mode = DISASSEM_WIN;
+		  layout_def->split = FALSE;
+		  break;
+		default:
+		  break;
 		}
-	      /*
-	       * Now update the window content.
-	       */
-	      if (!regs_populate 
-		  && (new_layout == SRC_DATA_COMMAND 
-		      || new_layout == DISASSEM_DATA_COMMAND))
-		tui_display_all_data ();
-
-	      tui_update_source_windows_with_addr (gdbarch, addr);
 	    }
+	  /*
+	   * Now update the window content.
+	   */
+	  if (!regs_populate
+	      && (new_layout == SRC_DATA_COMMAND
+		  || new_layout == DISASSEM_DATA_COMMAND))
+	    tui_display_all_data ();
+
+	  tui_update_source_windows_with_addr (gdbarch, addr);
+
 	  if (regs_populate)
 	    {
-              tui_show_registers (TUI_DATA_WIN->detail.data_display_info.current_group);
+	      struct reggroup *group =
+		TUI_DATA_WIN->detail.data_display_info.current_group;
+	      tui_show_registers (group);
 	    }
 	}
     }
@@ -370,7 +350,6 @@ tui_default_win_viewport_height (enum tui_win_type type,
   return h;
 }
 
-
 /* Function to initialize gdb commands, for tui window layout
    manipulation.  */
 
@@ -413,7 +392,6 @@ tui_set_layout_for_display_command (const char *layout_name)
       int i;
       char *buf_ptr;
       enum tui_layout_type new_layout = UNDEFINED_LAYOUT;
-      enum tui_register_display_type dpy_type = TUI_UNDEFINED_REGS;
       enum tui_layout_type cur_layout = tui_current_layout ();
 
       buf_ptr = (char *) xstrdup (layout_name);
@@ -421,8 +399,7 @@ tui_set_layout_for_display_command (const char *layout_name)
 	buf_ptr[i] = toupper (buf_ptr[i]);
 
       /* First check for ambiguous input.  */
-      if (strlen (buf_ptr) <= 1 
-	  && (*buf_ptr == 'S' || *buf_ptr == '$'))
+      if (strlen (buf_ptr) <= 1 && *buf_ptr == 'S')
 	{
 	  warning (_("Ambiguous command input."));
 	  status = TUI_FAILURE;
@@ -435,59 +412,13 @@ tui_set_layout_for_display_command (const char *layout_name)
 	    new_layout = DISASSEM_COMMAND;
 	  else if (subset_compare (buf_ptr, "SPLIT"))
 	    new_layout = SRC_DISASSEM_COMMAND;
-	  else if (subset_compare (buf_ptr, "REGS") 
-		   || subset_compare (buf_ptr, TUI_GENERAL_SPECIAL_REGS_NAME)
-		   || subset_compare (buf_ptr, TUI_GENERAL_REGS_NAME)
-		   || subset_compare (buf_ptr, TUI_FLOAT_REGS_NAME)
-		   || subset_compare (buf_ptr, TUI_SPECIAL_REGS_NAME))
+	  else if (subset_compare (buf_ptr, "REGS"))
 	    {
-	      if (cur_layout == SRC_COMMAND 
+	      if (cur_layout == SRC_COMMAND
 		  || cur_layout == SRC_DATA_COMMAND)
 		new_layout = SRC_DATA_COMMAND;
 	      else
 		new_layout = DISASSEM_DATA_COMMAND;
-
-	      /* Could ifdef out the following code. when compile with
-		 -z, there are null pointer references that cause a
-		 core dump if 'layout regs' is the first layout
-		 command issued by the user. HP has asked us to hook
-		 up this code.  - edie epstein  */
-	      if (subset_compare (buf_ptr, TUI_FLOAT_REGS_NAME))
-		{
-		  if (TUI_DATA_WIN->detail.data_display_info.regs_display_type
-		      != TUI_SFLOAT_REGS
-		      && TUI_DATA_WIN->detail.data_display_info.regs_display_type
-		      != TUI_DFLOAT_REGS)
-		    dpy_type = TUI_SFLOAT_REGS;
-		  else
-		    dpy_type =
-		      TUI_DATA_WIN->detail.data_display_info.regs_display_type;
-		}
-	      else if (subset_compare (buf_ptr,
-				      TUI_GENERAL_SPECIAL_REGS_NAME))
-		dpy_type = TUI_GENERAL_AND_SPECIAL_REGS;
-	      else if (subset_compare (buf_ptr, TUI_GENERAL_REGS_NAME))
-		dpy_type = TUI_GENERAL_REGS;
-	      else if (subset_compare (buf_ptr, TUI_SPECIAL_REGS_NAME))
-		dpy_type = TUI_SPECIAL_REGS;
-	      else if (TUI_DATA_WIN)
-		{
-		  if (TUI_DATA_WIN->detail.data_display_info.regs_display_type
-		      != TUI_UNDEFINED_REGS)
-		    dpy_type
-		      = TUI_DATA_WIN->detail.data_display_info.regs_display_type;
-		  else
-		    dpy_type = TUI_GENERAL_REGS;
-		}
-
-	      /* End of potential ifdef.
-	       */
-
-	      /* If ifdefed out code above, then assume that the user
-		 wishes to display the general purpose registers .
-	      */
-
-	      /* dpy_type = TUI_GENERAL_REGS; */
 	    }
 	  else if (subset_compare (buf_ptr, "NEXT"))
 	    new_layout = next_layout ();
@@ -496,7 +427,7 @@ tui_set_layout_for_display_command (const char *layout_name)
 	  else
 	    status = TUI_FAILURE;
 
-	  tui_set_layout (new_layout, dpy_type);
+	  tui_set_layout (new_layout);
 	}
       xfree (buf_ptr);
     }
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index 6dedb7d..623d254 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -30,7 +30,6 @@ extern int tui_default_win_height (enum tui_win_type,
 				   enum tui_layout_type);
 extern int tui_default_win_viewport_height (enum tui_win_type,
 					    enum tui_layout_type);
-extern enum tui_status tui_set_layout (enum tui_layout_type,
-				       enum tui_register_display_type);
+extern enum tui_status tui_set_layout (enum tui_layout_type);
 
 #endif /*TUI_LAYOUT_H */
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index b04e106..308e7ae 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -150,7 +150,6 @@ tui_rl_change_windows (int notused1, int notused2)
   if (tui_active)
     {
       enum tui_layout_type new_layout;
-      enum tui_register_display_type regs_type = TUI_UNDEFINED_REGS;
 
       new_layout = tui_current_layout ();
 
@@ -182,7 +181,7 @@ tui_rl_change_windows (int notused1, int notused2)
 	  new_layout = SRC_COMMAND;
 	  break;
 	}
-      tui_set_layout (new_layout, regs_type);
+      tui_set_layout (new_layout);
     }
   return 0;
 }
@@ -198,7 +197,6 @@ tui_rl_delete_other_windows (int notused1, int notused2)
   if (tui_active)
     {
       enum tui_layout_type new_layout;
-      enum tui_register_display_type regs_type = TUI_UNDEFINED_REGS;
 
       new_layout = tui_current_layout ();
 
@@ -217,7 +215,7 @@ tui_rl_delete_other_windows (int notused1, int notused2)
 	  new_layout = DISASSEM_COMMAND;
 	  break;
 	}
-      tui_set_layout (new_layout, regs_type);
+      tui_set_layout (new_layout);
     }
   return 0;
 }
@@ -464,7 +462,7 @@ tui_enable (void)
       def_prog_mode ();
 
       tui_show_frame_info (0);
-      tui_set_layout (SRC_COMMAND, TUI_UNDEFINED_REGS);
+      tui_set_layout (SRC_COMMAND);
       tui_set_win_focus_to (TUI_SRC_WIN);
       keypad (TUI_CMD_WIN->generic.handle, TRUE);
       wrefresh (TUI_CMD_WIN->generic.handle);
-- 
2.4.0

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

* [PATCH 3/4] gdb: Don't call tui_enable too early.
  2015-05-20 23:17 [PATCH 0/4] layout command changes Andrew Burgess
  2015-05-20 23:18 ` [PATCH 2/4] gdb: Add completer for layout command Andrew Burgess
  2015-05-20 23:18 ` [PATCH 4/4] gdb: Add cleanup to avoid memory leak on error Andrew Burgess
@ 2015-05-20 23:18 ` Andrew Burgess
  2015-05-20 23:18 ` [PATCH 1/4] gdb: Remove register class specific layout names Andrew Burgess
  2015-05-21  8:12 ` [PATCH 0/4] layout command changes Pedro Alves
  4 siblings, 0 replies; 14+ messages in thread
From: Andrew Burgess @ 2015-05-20 23:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Calling tui_enable too early in tui_layout_command can leave the tui in
an enabled state if the user has entered an invalid layout name.
Instead postpone the call to tui_enable until later in
tui_set_layout_for_display_command just before the layout is changed.

gdb/ChangeLog:

	* tui/tui-layout.c (tui_layout_command): Move call to tui_enable
	into ...
	(tui_set_layout_for_display_command): ...here, before calling
	tui_set_layout.  Only set the layout if gdb has not already
	entered the TUI_FAILURE state.
---
 gdb/ChangeLog        |  8 ++++++++
 gdb/tui/tui-layout.c | 10 ++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 51d2bfc..8ea1a5e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* tui/tui-layout.c (tui_layout_command): Move call to tui_enable
+	into ...
+	(tui_set_layout_for_display_command): ...here, before calling
+	tui_set_layout.  Only set the layout if gdb has not already
+	entered the TUI_FAILURE state.
+
+2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* tui/tui-layout.c (layout_completer): New function.
 	(_initialize_tui_layout): Set completer on layout command.
 
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 45fa575..2e950f7 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -460,7 +460,12 @@ tui_set_layout_for_display_command (const char *layout_name)
 	  else
 	    status = TUI_FAILURE;
 
-	  tui_set_layout (new_layout);
+	  if (status == TUI_SUCCESS)
+	    {
+	      /* Make sure the curses mode is enabled.  */
+	      tui_enable ();
+	      tui_set_layout (new_layout);
+	    }
 	}
       xfree (buf_ptr);
     }
@@ -509,9 +514,6 @@ extract_display_start_addr (struct gdbarch **gdbarch_p, CORE_ADDR *addr_p)
 static void
 tui_layout_command (char *arg, int from_tty)
 {
-  /* Make sure the curses mode is enabled.  */
-  tui_enable ();
-
   /* Switch to the selected layout.  */
   if (tui_set_layout_for_display_command (arg) != TUI_SUCCESS)
     warning (_("Invalid layout specified.\n%s"), LAYOUT_USAGE);
-- 
2.4.0

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

* [PATCH 4/4] gdb: Add cleanup to avoid memory leak on error.
  2015-05-20 23:17 [PATCH 0/4] layout command changes Andrew Burgess
  2015-05-20 23:18 ` [PATCH 2/4] gdb: Add completer for layout command Andrew Burgess
@ 2015-05-20 23:18 ` Andrew Burgess
  2015-05-20 23:18 ` [PATCH 3/4] gdb: Don't call tui_enable too early Andrew Burgess
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Andrew Burgess @ 2015-05-20 23:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Use cleanup to avoid leaking memory if an error occurs during tui
start up.

gdb/ChangeLog:

	* tui/tui-layout.c (tui_set_layout_for_display_command): Ensure
	buf_ptr is freed.
---
 gdb/ChangeLog        | 5 +++++
 gdb/tui/tui-layout.c | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8ea1a5e..78e5654 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* tui/tui-layout.c (tui_set_layout_for_display_command): Ensure
+	buf_ptr is freed.
+
+2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* tui/tui-layout.c (tui_layout_command): Move call to tui_enable
 	into ...
 	(tui_set_layout_for_display_command): ...here, before calling
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 2e950f7..0614b2c 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -426,10 +426,12 @@ tui_set_layout_for_display_command (const char *layout_name)
       char *buf_ptr;
       enum tui_layout_type new_layout = UNDEFINED_LAYOUT;
       enum tui_layout_type cur_layout = tui_current_layout ();
+      struct cleanup *old_chain;
 
       buf_ptr = (char *) xstrdup (layout_name);
       for (i = 0; (i < strlen (layout_name)); i++)
 	buf_ptr[i] = toupper (buf_ptr[i]);
+      old_chain = make_cleanup (xfree, buf_ptr);
 
       /* First check for ambiguous input.  */
       if (strlen (buf_ptr) <= 1 && *buf_ptr == 'S')
@@ -467,7 +469,7 @@ tui_set_layout_for_display_command (const char *layout_name)
 	      tui_set_layout (new_layout);
 	    }
 	}
-      xfree (buf_ptr);
+      do_cleanups (old_chain);
     }
   else
     status = TUI_FAILURE;
-- 
2.4.0

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

* [PATCH 2/4] gdb: Add completer for layout command.
  2015-05-20 23:17 [PATCH 0/4] layout command changes Andrew Burgess
@ 2015-05-20 23:18 ` Andrew Burgess
  2015-05-21  0:25   ` Keith Seitz
  2015-05-20 23:18 ` [PATCH 4/4] gdb: Add cleanup to avoid memory leak on error Andrew Burgess
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2015-05-20 23:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Add layout name completion for the layout command.

gdb/ChangeLog:

	* tui/tui-layout.c (layout_completer): New function.
	(_initialize_tui_layout): Set completer on layout command.

gdb/testsuite/ChangeLog:

	* gdb.base/completion.exp: Add test for completion of layout
	names.
---
 gdb/ChangeLog                         |  5 +++++
 gdb/testsuite/ChangeLog               |  5 +++++
 gdb/testsuite/gdb.base/completion.exp | 19 +++++++++++++++++++
 gdb/tui/tui-layout.c                  | 35 ++++++++++++++++++++++++++++++++++-
 4 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 40c70e7..51d2bfc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* tui/tui-layout.c (layout_completer): New function.
+	(_initialize_tui_layout): Set completer on layout command.
+
+2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* tui/tui-layout.c (tui_set_layout): Remove
 	tui_register_display_type parameter.  Remove all checking of this
 	parameter, and reindent function.  Update header comment.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3a947eb..15dae61 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.base/completion.exp: Add test for completion of layout
+	names.
+
+2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* lib/gdb.exp (skip_tui_tests): New proc.
 	* gdb.base/tui-layout.exp: Check skip_tui_tests.
 
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index f77bfe2..4c31bfc 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -859,3 +859,22 @@ gdb_test_multiple "" "$test" {
 	pass "$test"
     }
 }
+
+gdb_test_no_output "set max-completions unlimited"
+
+if {![skip_tui_tests]} {
+    set test "test completion of layout names"
+    send_gdb "layout\t\t\t"
+    gdb_test_multiple "" "$test" {
+	-re "asm *next *prev *regs *split *src *\r\n$gdb_prompt layout $" {
+	    pass "$test"
+	}
+    }
+    send_gdb "\003"
+    set test "quit command input after testing layout completion"
+    gdb_test_multiple "" "$test" {
+	-re "$gdb_prompt $" {
+	    pass "$test"
+	}
+    }
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 44aca5d..45fa575 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -350,6 +350,36 @@ tui_default_win_viewport_height (enum tui_win_type type,
   return h;
 }
 
+/* Complete possible layout names.  TEXT is the complete text entered so
+   far, WORD is the word currently being completed.  */
+
+static VEC (char_ptr) *
+layout_completer (struct cmd_list_element *ignore,
+		  const char *text, const char *word)
+{
+  VEC (char_ptr) *return_val = NULL;
+  const char **tmp, *p;
+  size_t len;
+
+  static const char *layout_names [] =
+    { "src", "asm", "split", "regs", "next", "prev", NULL };
+
+  gdb_assert (text != NULL);
+  p = strchr (text, ' ');
+  len = (p == NULL) ? strlen (text) : p - text;
+
+  for (tmp = layout_names; *tmp != NULL; ++tmp)
+    {
+      if (strncmp (text, *tmp, len) == 0)
+	{
+	  char *str = xstrdup (*tmp);
+	  VEC_safe_push (char_ptr, return_val, str);
+	}
+    }
+
+  return return_val;
+}
+
 /* Function to initialize gdb commands, for tui window layout
    manipulation.  */
 
@@ -359,7 +389,9 @@ extern initialize_file_ftype _initialize_tui_layout;
 void
 _initialize_tui_layout (void)
 {
-  add_com ("layout", class_tui, tui_layout_command, _("\
+  struct cmd_list_element *cmd;
+
+  cmd = add_com ("layout", class_tui, tui_layout_command, _("\
 Change the layout of windows.\n\
 Usage: layout prev | next | <layout_name> \n\
 Layout names are:\n\
@@ -372,6 +404,7 @@ Layout names are:\n\
            source/assembly/command (split) is displayed, \n\
            the register window is displayed with \n\
            the window that has current logical focus.\n"));
+  set_cmd_completer (cmd, layout_completer);
 }
 
 
-- 
2.4.0

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

* Re: [PATCH 2/4] gdb: Add completer for layout command.
  2015-05-20 23:18 ` [PATCH 2/4] gdb: Add completer for layout command Andrew Burgess
@ 2015-05-21  0:25   ` Keith Seitz
  2015-05-21  7:10     ` Andrew Burgess
  0 siblings, 1 reply; 14+ messages in thread
From: Keith Seitz @ 2015-05-21  0:25 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 05/20/2015 04:17 PM, Andrew Burgess wrote:
> Add layout name completion for the layout command.
> 
> gdb/ChangeLog:
> 
> 	* tui/tui-layout.c (layout_completer): New function.
> 	(_initialize_tui_layout): Set completer on layout command.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/completion.exp: Add test for completion of layout
> 	names.
> ---
>  gdb/ChangeLog                         |  5 +++++
>  gdb/testsuite/ChangeLog               |  5 +++++
>  gdb/testsuite/gdb.base/completion.exp | 19 +++++++++++++++++++
>  gdb/tui/tui-layout.c                  | 35 ++++++++++++++++++++++++++++++++++-
>  4 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 40c70e7..51d2bfc 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,10 @@
>  2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
>  
> +	* tui/tui-layout.c (layout_completer): New function.
> +	(_initialize_tui_layout): Set completer on layout command.
> +
> +2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
>  	* tui/tui-layout.c (tui_set_layout): Remove
>  	tui_register_display_type parameter.  Remove all checking of this
>  	parameter, and reindent function.  Update header comment.
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 3a947eb..15dae61 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,5 +1,10 @@
>  2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
>  
> +	* gdb.base/completion.exp: Add test for completion of layout
> +	names.
> +
> +2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
>  	* lib/gdb.exp (skip_tui_tests): New proc.
>  	* gdb.base/tui-layout.exp: Check skip_tui_tests.
>  
> diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
> index f77bfe2..4c31bfc 100644
> --- a/gdb/testsuite/gdb.base/completion.exp
> +++ b/gdb/testsuite/gdb.base/completion.exp
> @@ -859,3 +859,22 @@ gdb_test_multiple "" "$test" {
>  	pass "$test"
>      }
>  }
> +
> +gdb_test_no_output "set max-completions unlimited"
> +
> +if {![skip_tui_tests]} {
> +    set test "test completion of layout names"
> +    send_gdb "layout\t\t\t"
> +    gdb_test_multiple "" "$test" {
> +	-re "asm *next *prev *regs *split *src *\r\n$gdb_prompt layout $" {
> +	    pass "$test"
> +	}
> +    }
> +    send_gdb "\003"
> +    set test "quit command input after testing layout completion"
> +    gdb_test_multiple "" "$test" {
> +	-re "$gdb_prompt $" {
> +	    pass "$test"
> +	}
> +    }
> +}
> diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
> index 44aca5d..45fa575 100644
> --- a/gdb/tui/tui-layout.c
> +++ b/gdb/tui/tui-layout.c
> @@ -350,6 +350,36 @@ tui_default_win_viewport_height (enum tui_win_type type,
>    return h;
>  }
>  
> +/* Complete possible layout names.  TEXT is the complete text entered so
> +   far, WORD is the word currently being completed.  */
> +
> +static VEC (char_ptr) *
> +layout_completer (struct cmd_list_element *ignore,
> +		  const char *text, const char *word)
> +{
> +  VEC (char_ptr) *return_val = NULL;
> +  const char **tmp, *p;
> +  size_t len;
> +
> +  static const char *layout_names [] =
> +    { "src", "asm", "split", "regs", "next", "prev", NULL };
> +
> +  gdb_assert (text != NULL);
> +  p = strchr (text, ' ');
> +  len = (p == NULL) ? strlen (text) : p - text;
> +
> +  for (tmp = layout_names; *tmp != NULL; ++tmp)
> +    {
> +      if (strncmp (text, *tmp, len) == 0)
> +	{
> +	  char *str = xstrdup (*tmp);
> +	  VEC_safe_push (char_ptr, return_val, str);
> +	}
> +    }
> +

One quick observation (having seen my fair share of completion functions
recently)... Most completer functions are using complete_on_enum to do
this sort of thing. Will that work here?

Keith


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

* Re: [PATCH 2/4] gdb: Add completer for layout command.
  2015-05-21  0:25   ` Keith Seitz
@ 2015-05-21  7:10     ` Andrew Burgess
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Burgess @ 2015-05-21  7:10 UTC (permalink / raw)
  To: gdb-patches; +Cc: Keith Seitz

* Keith Seitz <keiths@redhat.com> [2015-05-20 17:25:14 -0700]:

> One quick observation (having seen my fair share of completion functions
> recently)... Most completer functions are using complete_on_enum to do
> this sort of thing. Will that work here?

Indeed it will, I never knew this even existed :)

New simpler version of the patch below.

Thanks,
Andrew

--

gdb: Add completer for layout command.

Add layout name completion for the layout command.

gdb/ChangeLog:

	* tui/tui-layout.c (layout_completer): New function.
	(_initialize_tui_layout): Set completer on layout command.

gdb/testsuite/ChangeLog:

	* gdb.base/completion.exp: Add test for completion of layout
	names.

--

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 40c70e7..51d2bfc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* tui/tui-layout.c (layout_completer): New function.
+	(_initialize_tui_layout): Set completer on layout command.
+
+2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* tui/tui-layout.c (tui_set_layout): Remove
 	tui_register_display_type parameter.  Remove all checking of this
 	parameter, and reindent function.  Update header comment.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3a947eb..15dae61 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.base/completion.exp: Add test for completion of layout
+	names.
+
+2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* lib/gdb.exp (skip_tui_tests): New proc.
 	* gdb.base/tui-layout.exp: Check skip_tui_tests.
 
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index f77bfe2..4c31bfc 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -859,3 +859,22 @@ gdb_test_multiple "" "$test" {
 	pass "$test"
     }
 }
+
+gdb_test_no_output "set max-completions unlimited"
+
+if {![skip_tui_tests]} {
+    set test "test completion of layout names"
+    send_gdb "layout\t\t\t"
+    gdb_test_multiple "" "$test" {
+	-re "asm *next *prev *regs *split *src *\r\n$gdb_prompt layout $" {
+	    pass "$test"
+	}
+    }
+    send_gdb "\003"
+    set test "quit command input after testing layout completion"
+    gdb_test_multiple "" "$test" {
+	-re "$gdb_prompt $" {
+	    pass "$test"
+	}
+    }
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 44aca5d..600f0de 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -350,6 +350,19 @@ tui_default_win_viewport_height (enum tui_win_type type,
   return h;
 }
 
+/* Complete possible layout names.  TEXT is the complete text entered so
+   far, WORD is the word currently being completed.  */
+
+static VEC (char_ptr) *
+layout_completer (struct cmd_list_element *ignore,
+		  const char *text, const char *word)
+{
+  static const char *layout_names [] =
+    { "src", "asm", "split", "regs", "next", "prev", NULL };
+
+  return complete_on_enum (layout_names, text, word);
+}
+
 /* Function to initialize gdb commands, for tui window layout
    manipulation.  */
 
@@ -359,7 +372,9 @@ extern initialize_file_ftype _initialize_tui_layout;
 void
 _initialize_tui_layout (void)
 {
-  add_com ("layout", class_tui, tui_layout_command, _("\
+  struct cmd_list_element *cmd;
+
+  cmd = add_com ("layout", class_tui, tui_layout_command, _("\
 Change the layout of windows.\n\
 Usage: layout prev | next | <layout_name> \n\
 Layout names are:\n\
@@ -372,6 +387,7 @@ Layout names are:\n\
            source/assembly/command (split) is displayed, \n\
            the register window is displayed with \n\
            the window that has current logical focus.\n"));
+  set_cmd_completer (cmd, layout_completer);
 }
 
 

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

* Re: [PATCH 0/4] layout command changes
  2015-05-20 23:17 [PATCH 0/4] layout command changes Andrew Burgess
                   ` (3 preceding siblings ...)
  2015-05-20 23:18 ` [PATCH 1/4] gdb: Remove register class specific layout names Andrew Burgess
@ 2015-05-21  8:12 ` Pedro Alves
  2015-05-21  8:35   ` Pedro Alves
  4 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2015-05-21  8:12 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 05/21/2015 12:17 AM, Andrew Burgess wrote:
> This patch set replaces an earlier patch I posted here:
>   https://sourceware.org/ml/gdb-patches/2015-04/msg00185.html
> 
> In the previous patch I had to jump through some hoops in order to
> support completion of the layout names like $FREGS.  This was pretty
> annoying as I had not realised these layouts existed until I started
> writting the completer code...
> 
> ...but it turns out that those layout names don't work anyway, and
> have not done so for some time.  I didn't figure out exactly when they
> broke, but I believe they were broken in 6.8.
> 
> Still, it doesn't matter, as we have the 'tui regs' command, which
> does work, and does allow the register set displayed in tui to be
> changed.  This is for the best anyway (I think), personally, I felt
> that managing both the layout, and the choice of register set all from
> the layout command was too much overloading.
> 
> The first patch in this series removes the $FREGS style register set
> names from the layout command, and cleans up all of the code relating
> to them.

Looks like this was really meant to switch to the matching registers
layout when the user did "display $fpregs", etc. instead of manually
specifying that layout.  We have:

static void
display_command (char *arg, int from_tty)
{
  struct format_data fmt;
  struct expression *expr;
  struct display *newobj;
  int display_it = 1;
  const char *exp = arg;

#if defined(TUI)
  /* NOTE: cagney/2003-02-13 The `tui_active' was previously
     `tui_version'.  */
  if (tui_active && exp != NULL && *exp == '$')
    display_it = (tui_set_layout_for_display_command (exp) == TUI_FAILURE);
#endif

Doesn't your series effectively make this bit in display_command dead?
(while before it would switch on the registers layout).  (We should probably
rename tui_set_layout_for_display_command too.)

I had never noticed these special register layouts before either.  I'm not
at all adverse to removing them.  Not all expressions that start with $ are
registers, and probably a better idea would be to have
a separate "displays" window (so displays would go to that window
instead of the command window when the TUI is active), so that the tui
could neatly show watched variables/random expressions too.

> 
> The second patch is a much simpler version of command completion
> support for layout names.
> 
> The third and forth patches fix small tui related issues that I
> spotted during testing.


Thanks,
Pedro Alves

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

* Re: [PATCH 0/4] layout command changes
  2015-05-21  8:12 ` [PATCH 0/4] layout command changes Pedro Alves
@ 2015-05-21  8:35   ` Pedro Alves
  0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2015-05-21  8:35 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 05/21/2015 09:12 AM, Pedro Alves wrote:
> On 05/21/2015 12:17 AM, Andrew Burgess wrote:
>> This patch set replaces an earlier patch I posted here:
>>   https://sourceware.org/ml/gdb-patches/2015-04/msg00185.html
>>
>> In the previous patch I had to jump through some hoops in order to
>> support completion of the layout names like $FREGS.  This was pretty
>> annoying as I had not realised these layouts existed until I started
>> writting the completer code...
>>
>> ...but it turns out that those layout names don't work anyway, and
>> have not done so for some time.  I didn't figure out exactly when they
>> broke, but I believe they were broken in 6.8.
>>
>> Still, it doesn't matter, as we have the 'tui regs' command, which
>> does work, and does allow the register set displayed in tui to be
>> changed.  This is for the best anyway (I think), personally, I felt
>> that managing both the layout, and the choice of register set all from
>> the layout command was too much overloading.
>>
>> The first patch in this series removes the $FREGS style register set
>> names from the layout command, and cleans up all of the code relating
>> to them.
> 
> Looks like this was really meant to switch to the matching registers
> layout when the user did "display $fpregs", etc. instead of manually
> specifying that layout.  We have:
> 
> static void
> display_command (char *arg, int from_tty)
> {
>   struct format_data fmt;
>   struct expression *expr;
>   struct display *newobj;
>   int display_it = 1;
>   const char *exp = arg;
> 
> #if defined(TUI)
>   /* NOTE: cagney/2003-02-13 The `tui_active' was previously
>      `tui_version'.  */
>   if (tui_active && exp != NULL && *exp == '$')
>     display_it = (tui_set_layout_for_display_command (exp) == TUI_FAILURE);
> #endif
> 
> Doesn't your series effectively make this bit in display_command dead?
> (while before it would switch on the registers layout).  (We should probably
> rename tui_set_layout_for_display_command too.)
> 
> I had never noticed these special register layouts before either.  I'm not
> at all adverse to removing them.  Not all expressions that start with $ are
> registers, and probably a better idea would be to have
> a separate "displays" window (so displays would go to that window
> instead of the command window when the TUI is active), so that the tui
> could neatly show watched variables/random expressions too.

I read the whole series now (the complete_on_enum version), and it otherwise
looks all good to me.

Thanks,
Pedro Alves

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

* Re: [PATCH 1/4] gdb: Remove register class specific layout names.
  2015-05-20 23:18 ` [PATCH 1/4] gdb: Remove register class specific layout names Andrew Burgess
@ 2015-05-21  8:42   ` Pedro Alves
  2015-05-21 11:33     ` Andrew Burgess
  2015-05-21 12:25   ` Andrew Burgess
  1 sibling, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2015-05-21  8:42 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 05/21/2015 12:17 AM, Andrew Burgess wrote:
> 
> Second there is already the command 'tui reg GROUP' command to set the
> displayed register set to GROUP, so making the layout command also
> control the register set feels like unnecessary overloading of the
> layout command.

(A tangent: I was playing with this a bit now, and found it quite odd
that there's a "tui reg next" command here, but
no "tui reg previous"...)

Thanks,
Pedro Alves

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

* Re: [PATCH 1/4] gdb: Remove register class specific layout names.
  2015-05-21  8:42   ` Pedro Alves
@ 2015-05-21 11:33     ` Andrew Burgess
  2015-05-21 11:34       ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2015-05-21 11:33 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

* Pedro Alves <palves@redhat.com> [2015-05-21 09:42:17 +0100]:

> On 05/21/2015 12:17 AM, Andrew Burgess wrote:
> > 
> > Second there is already the command 'tui reg GROUP' command to set the
> > displayed register set to GROUP, so making the layout command also
> > control the register set feels like unnecessary overloading of the
> > layout command.
> 
> (A tangent: I was playing with this a bit now, and found it quite odd
> that there's a "tui reg next" command here, but
> no "tui reg previous"...)

Indeed, this is on my list of things to look at next (unless you're
already fixing it).

You'll also notice, at least on x86-64 that if you use 'tui reg next'
you get access to more register sets that are offered in the
tab-completion mechanism.  This too is something I plan to address in
the next series.

Thanks,
Andrew

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

* Re: [PATCH 1/4] gdb: Remove register class specific layout names.
  2015-05-21 11:33     ` Andrew Burgess
@ 2015-05-21 11:34       ` Pedro Alves
  0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2015-05-21 11:34 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

On 05/21/2015 12:33 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2015-05-21 09:42:17 +0100]:
> 
>> On 05/21/2015 12:17 AM, Andrew Burgess wrote:
>>>
>>> Second there is already the command 'tui reg GROUP' command to set the
>>> displayed register set to GROUP, so making the layout command also
>>> control the register set feels like unnecessary overloading of the
>>> layout command.
>>
>> (A tangent: I was playing with this a bit now, and found it quite odd
>> that there's a "tui reg next" command here, but
>> no "tui reg previous"...)
> 
> Indeed, this is on my list of things to look at next (unless you're
> already fixing it).

I'm not fixing it.

> You'll also notice, at least on x86-64 that if you use 'tui reg next'
> you get access to more register sets that are offered in the
> tab-completion mechanism.  This too is something I plan to address in
> the next series.

Sounds all great, thanks!

-- 
Pedro Alves

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

* Re: [PATCH 1/4] gdb: Remove register class specific layout names.
  2015-05-20 23:18 ` [PATCH 1/4] gdb: Remove register class specific layout names Andrew Burgess
  2015-05-21  8:42   ` Pedro Alves
@ 2015-05-21 12:25   ` Andrew Burgess
  2015-05-21 13:17     ` Pedro Alves
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2015-05-21 12:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves


* Pedro Alves <palves@redhat.com> [2015-05-21 09:12:45 +0100]:

> On 05/21/2015 12:17 AM, Andrew Burgess wrote:
> >
> > The first patch in this series removes the $FREGS style register set
> > names from the layout command, and cleans up all of the code relating
> > to them.
>
> Looks like this was really meant to switch to the matching registers
> layout when the user did "display $fpregs", etc. instead of manually
> specifying that layout.  We have:
> 
> static void
> display_command (char *arg, int from_tty)
> {
>   struct format_data fmt;
>   struct expression *expr;
>   struct display *newobj;
>   int display_it = 1;
>   const char *exp = arg;
> 
> #if defined(TUI)
>   /* NOTE: cagney/2003-02-13 The `tui_active' was previously
>      `tui_version'.  */
>   if (tui_active && exp != NULL && *exp == '$')
>     display_it = (tui_set_layout_for_display_command (exp) == TUI_FAILURE);
> #endif
> 
> Doesn't your series effectively make this bit in display_command dead?
> (while before it would switch on the registers layout).

Sorry for missing that use.

You're right, it's certainly dead now, though before my patch it still
didn't do anything, passing the '$FREGS', etc names to
tui_set_layout_for_display_command would just cause the regs display
to pop up with the default register set. (At least, that's all I
ever see, and I don't think anything else is possible.)

Given that it's not worked for a while, I propose to just remove the
above snippet (really having this in display seems nasty to me) and
just move to layout being for layout adjustment, and 'tui reg' to
change the displayed register set.

>                                                         (We should probably
> rename tui_set_layout_for_display_command too.)

Done, it's now tui_set_layout_by_name.

> I had never noticed these special register layouts before either.  I'm not
> at all adverse to removing them.  Not all expressions that start with $ are
> registers, and probably a better idea would be to have
> a separate "displays" window (so displays would go to that window
> instead of the command window when the TUI is active), so that the tui
> could neatly show watched variables/random expressions too.

Sounds like a good idea.

New patch below.

Thanks,
Andrew

--

gdb: Remove register class specific layout names.

The layout command supports the layout names $FREGS, $GREGS, $SREGS,
and $REGS. The intention of these layout names was to display the tui
register window with a specific set of registers.

First, these layout names no longer work, and haven't for a while, using
any of them will just result in switching to the general register view.

Second there is already the command 'tui reg GROUP' command to set the
displayed register set to GROUP, so making the layout command also
control the register set feels like unnecessary overloading of the
layout command.

This commit removes all code relating to supporting the register set
specific names from the layout command.  Afterwards the user can select
an available layout using the layout command, and control the choice of
register set using the 'tui reg GROUP' command.

gdb/ChangeLog:

	* tui/tui-layout.c (tui_set_layout): Remove
	tui_register_display_type parameter.  Remove all checking of this
	parameter, and reindent function.  Update header comment.
	(tui_set_layout_for_display_command): Rename to...
	(tui_set_layout_by_name): ...this, and don't check for different
	register class types, don't pass a tui_register_display_type to
	tui_set_layout.  Update header comment.
	(layout_names): Remove register set specific names.
	* tui/tui-layout.h (tui_set_layout): Remove
	tui_register_display_type parameter.
	* tui/tui.c (tui_rl_change_windows): Don't pass a
	tui_register_display_type to tui_set_layout.
	(tui_rl_delete_other_windows): Likewise.
	(tui_enable): Likewise.
	* tui/tui-data.h (TUI_FLOAT_REGS_NAME): Remove.
	(TUI_FLOAT_REGS_NAME_LOWER): Remove.
	(TUI_GENERAL_REGS_NAME): Remove.
	(TUI_GENERAL_REGS_NAME_LOWER): Remove.
	(TUI_SPECIAL_REGS_NAME): Remove.
	(TUI_SPECIAL_REGS_NAME_LOWER): Remove.
	(TUI_GENERAL_SPECIAL_REGS_NAME): Remove.
	(TUI_GENERAL_SPECIAL_REGS_NAME_LOWER): Remove.
	(enum tui_register_display_type): Remove.
	(struct tui_layout_def): Remove regs_display_type and
	float_regs_display_type fields.
	(struct tui_data_info): Remove regs_display_type field.
	(tui_layout_command): Use new name for
	tui_set_layout_for_display_command.
	* tui/tui-data.c (layout_def): Don't initialise removed fields.
	(tui_clear_win_detail): Don't initialise removed fields of
	win_info.
	* tui/tui-regs.c (tui_show_registers): Use new name for
	tui_set_layout_for_display_command.
	* tui/tui.h (tui_set_layout_for_display_command): Rename
	declaration to...
	(tui_set_layout_by_name): ...this.
	* printcmd.c (display_command): Remove tui related layout call,
	and reindent.

--

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 28d6b71..e58c45b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,44 @@
+2015-05-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* tui/tui-layout.c (tui_set_layout): Remove
+	tui_register_display_type parameter.  Remove all checking of this
+	parameter, and reindent function.  Update header comment.
+	(tui_set_layout_for_display_command): Rename to...
+	(tui_set_layout_by_name): ...this, and don't check for different
+	register class types, don't pass a tui_register_display_type to
+	tui_set_layout.  Update header comment.
+	(layout_names): Remove register set specific names.
+	* tui/tui-layout.h (tui_set_layout): Remove
+	tui_register_display_type parameter.
+	* tui/tui.c (tui_rl_change_windows): Don't pass a
+	tui_register_display_type to tui_set_layout.
+	(tui_rl_delete_other_windows): Likewise.
+	(tui_enable): Likewise.
+	* tui/tui-data.h (TUI_FLOAT_REGS_NAME): Remove.
+	(TUI_FLOAT_REGS_NAME_LOWER): Remove.
+	(TUI_GENERAL_REGS_NAME): Remove.
+	(TUI_GENERAL_REGS_NAME_LOWER): Remove.
+	(TUI_SPECIAL_REGS_NAME): Remove.
+	(TUI_SPECIAL_REGS_NAME_LOWER): Remove.
+	(TUI_GENERAL_SPECIAL_REGS_NAME): Remove.
+	(TUI_GENERAL_SPECIAL_REGS_NAME_LOWER): Remove.
+	(enum tui_register_display_type): Remove.
+	(struct tui_layout_def): Remove regs_display_type and
+	float_regs_display_type fields.
+	(struct tui_data_info): Remove regs_display_type field.
+	(tui_layout_command): Use new name for
+	tui_set_layout_for_display_command.
+	* tui/tui-data.c (layout_def): Don't initialise removed fields.
+	(tui_clear_win_detail): Don't initialise removed fields of
+	win_info.
+	* tui/tui-regs.c (tui_show_registers): Use new name for
+	tui_set_layout_for_display_command.
+	* tui/tui.h (tui_set_layout_for_display_command): Rename
+	declaration to...
+	(tui_set_layout_by_name): ...this.
+	* printcmd.c (display_command): Remove tui related layout call,
+	and reindent.
+
 2015-05-20  Joel Brobecker  <brobecker@adacore.com>
 
 	* infrun.c (handle_inferior_event_1): Renames handle_inferior_event.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 0ae402f..a739a89 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1506,61 +1506,50 @@ display_command (char *arg, int from_tty)
   struct format_data fmt;
   struct expression *expr;
   struct display *newobj;
-  int display_it = 1;
   const char *exp = arg;
 
-#if defined(TUI)
-  /* NOTE: cagney/2003-02-13 The `tui_active' was previously
-     `tui_version'.  */
-  if (tui_active && exp != NULL && *exp == '$')
-    display_it = (tui_set_layout_for_display_command (exp) == TUI_FAILURE);
-#endif
-
-  if (display_it)
+  if (exp == 0)
     {
-      if (exp == 0)
-	{
-	  do_displays ();
-	  return;
-	}
+      do_displays ();
+      return;
+    }
 
-      if (*exp == '/')
-	{
-	  exp++;
-	  fmt = decode_format (&exp, 0, 0);
-	  if (fmt.size && fmt.format == 0)
-	    fmt.format = 'x';
-	  if (fmt.format == 'i' || fmt.format == 's')
-	    fmt.size = 'b';
-	}
-      else
-	{
-	  fmt.format = 0;
-	  fmt.size = 0;
-	  fmt.count = 0;
-	  fmt.raw = 0;
-	}
+  if (*exp == '/')
+    {
+      exp++;
+      fmt = decode_format (&exp, 0, 0);
+      if (fmt.size && fmt.format == 0)
+	fmt.format = 'x';
+      if (fmt.format == 'i' || fmt.format == 's')
+	fmt.size = 'b';
+    }
+  else
+    {
+      fmt.format = 0;
+      fmt.size = 0;
+      fmt.count = 0;
+      fmt.raw = 0;
+    }
 
-      innermost_block = NULL;
-      expr = parse_expression (exp);
+  innermost_block = NULL;
+  expr = parse_expression (exp);
 
-      newobj = (struct display *) xmalloc (sizeof (struct display));
+  newobj = (struct display *) xmalloc (sizeof (struct display));
 
-      newobj->exp_string = xstrdup (exp);
-      newobj->exp = expr;
-      newobj->block = innermost_block;
-      newobj->pspace = current_program_space;
-      newobj->next = display_chain;
-      newobj->number = ++display_number;
-      newobj->format = fmt;
-      newobj->enabled_p = 1;
-      display_chain = newobj;
+  newobj->exp_string = xstrdup (exp);
+  newobj->exp = expr;
+  newobj->block = innermost_block;
+  newobj->pspace = current_program_space;
+  newobj->next = display_chain;
+  newobj->number = ++display_number;
+  newobj->format = fmt;
+  newobj->enabled_p = 1;
+  display_chain = newobj;
 
-      if (from_tty)
-	do_one_display (newobj);
+  if (from_tty)
+    do_one_display (newobj);
 
-      dont_repeat ();
-    }
+  dont_repeat ();
 }
 
 static void
diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index ffd80d5..ed42c8d 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -44,9 +44,7 @@ static int default_tab_len = DEFAULT_TAB_LEN;
 static struct tui_win_info *win_with_focus = (struct tui_win_info *) NULL;
 static struct tui_layout_def layout_def = {
   SRC_WIN,			/* DISPLAY_MODE */
-  FALSE,			/* SPLIT */
-  TUI_UNDEFINED_REGS,		/* REGS_DISPLAY_TYPE */
-  TUI_SFLOAT_REGS};		/* FLOAT_REGS_DISPLAY_TYPE */
+  FALSE};			/* SPLIT */
 
 static int win_resized = FALSE;
 
@@ -224,8 +222,6 @@ tui_clear_win_detail (struct tui_win_info *win_info)
 	  win_info->detail.data_display_info.regs_content =
 	    (tui_win_content) NULL;
 	  win_info->detail.data_display_info.regs_content_count = 0;
-	  win_info->detail.data_display_info.regs_display_type =
-	    TUI_UNDEFINED_REGS;
 	  win_info->detail.data_display_info.regs_column_count = 1;
 	  win_info->detail.data_display_info.display_regs = FALSE;
 	  break;
@@ -544,8 +540,6 @@ init_win_info (struct tui_win_info *win_info)
       win_info->detail.data_display_info.data_content_count = 0;
       win_info->detail.data_display_info.regs_content = (tui_win_content) NULL;
       win_info->detail.data_display_info.regs_content_count = 0;
-      win_info->detail.data_display_info.regs_display_type =
-	TUI_UNDEFINED_REGS;
       win_info->detail.data_display_info.regs_column_count = 1;
       win_info->detail.data_display_info.display_regs = FALSE;
       win_info->detail.data_display_info.current_group = 0;
@@ -745,8 +739,6 @@ tui_free_window (struct tui_win_info *win_info)
 	  win_info->detail.data_display_info.data_content =
 	    (tui_win_content) NULL;
 	  win_info->detail.data_display_info.data_content_count = 0;
-	  win_info->detail.data_display_info.regs_display_type =
-	    TUI_UNDEFINED_REGS;
 	  win_info->detail.data_display_info.regs_column_count = 1;
 	  win_info->detail.data_display_info.display_regs = FALSE;
 	  win_info->generic.content = NULL;
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 7651efd..05263e3 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -92,15 +92,6 @@ struct tui_gen_win_info
 #define MAX_TARGET_WIDTH  10
 #define MAX_PID_WIDTH     19
 
-#define TUI_FLOAT_REGS_NAME                  "$FREGS"
-#define TUI_FLOAT_REGS_NAME_LOWER            "$fregs"
-#define TUI_GENERAL_REGS_NAME                "$GREGS"
-#define TUI_GENERAL_REGS_NAME_LOWER          "$gregs"
-#define TUI_SPECIAL_REGS_NAME                "$SREGS"
-#define TUI_SPECIAL_REGS_NAME_LOWER          "$sregs"
-#define TUI_GENERAL_SPECIAL_REGS_NAME        "$REGS"
-#define TUI_GENERAL_SPECIAL_REGS_NAME_LOWER  "$regs"
-
 /* Scroll direction enum.  */
 enum tui_scroll_direction
 {
@@ -139,17 +130,6 @@ enum tui_data_type
   TUI_STRUCT
 };
 
-/* Types of register displays.  */
-enum tui_register_display_type
-{
-  TUI_UNDEFINED_REGS,
-  TUI_GENERAL_REGS,
-  TUI_SFLOAT_REGS,
-  TUI_DFLOAT_REGS,
-  TUI_SPECIAL_REGS,
-  TUI_GENERAL_AND_SPECIAL_REGS
-};
-
 enum tui_line_or_address_kind
 {
   LOA_LINE,
@@ -172,8 +152,6 @@ struct tui_layout_def
 {
   enum tui_win_type display_mode;
   int split;
-  enum tui_register_display_type regs_display_type;
-  enum tui_register_display_type float_regs_display_type;
 };
 
 /* Elements in the Source/Disassembly Window.  */
@@ -263,7 +241,6 @@ struct tui_data_info
   int data_content_count;
   tui_win_content regs_content;	/* Start of regs display content.  */
   int regs_content_count;
-  enum tui_register_display_type regs_display_type;
   int regs_column_count;
   int display_regs;		/* Should regs be displayed at all?  */
   struct reggroup *current_group;
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 4c25d43..3a2be9b 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -122,18 +122,13 @@ show_layout (enum tui_layout_type layout)
 
 
 /* Function to set the layout to SRC_COMMAND, DISASSEM_COMMAND,
-   SRC_DISASSEM_COMMAND, SRC_DATA_COMMAND, or DISASSEM_DATA_COMMAND.
-   If the layout is SRC_DATA_COMMAND, DISASSEM_DATA_COMMAND, or
-   UNDEFINED_LAYOUT, then the data window is populated according to
-   regs_display_type.  */
+   SRC_DISASSEM_COMMAND, SRC_DATA_COMMAND, or DISASSEM_DATA_COMMAND.  */
 enum tui_status
-tui_set_layout (enum tui_layout_type layout_type,
-		enum tui_register_display_type regs_display_type)
+tui_set_layout (enum tui_layout_type layout_type)
 {
   enum tui_status status = TUI_SUCCESS;
 
-  if (layout_type != UNDEFINED_LAYOUT 
-      || regs_display_type != TUI_UNDEFINED_REGS)
+  if (layout_type != UNDEFINED_LAYOUT)
     {
       enum tui_layout_type cur_layout = tui_current_layout (),
 	new_layout = UNDEFINED_LAYOUT;
@@ -145,113 +140,98 @@ tui_set_layout (enum tui_layout_type layout_type,
 
       extract_display_start_addr (&gdbarch, &addr);
 
-      if (layout_type == UNDEFINED_LAYOUT
-	  && regs_display_type != TUI_UNDEFINED_REGS)
-	{
-	  if (cur_layout == SRC_DISASSEM_COMMAND)
-	    new_layout = DISASSEM_DATA_COMMAND;
-	  else if (cur_layout == SRC_COMMAND 
-		   || cur_layout == SRC_DATA_COMMAND)
-	    new_layout = SRC_DATA_COMMAND;
-	  else if (cur_layout == DISASSEM_COMMAND 
-		   || cur_layout == DISASSEM_DATA_COMMAND)
-	    new_layout = DISASSEM_DATA_COMMAND;
-	}
-      else
-	new_layout = layout_type;
+      new_layout = layout_type;
 
-      regs_populate = (new_layout == SRC_DATA_COMMAND 
-		       || new_layout == DISASSEM_DATA_COMMAND 
-		       || regs_display_type != TUI_UNDEFINED_REGS);
-      if (new_layout != cur_layout
-	  || regs_display_type != TUI_UNDEFINED_REGS)
+      regs_populate = (new_layout == SRC_DATA_COMMAND
+		       || new_layout == DISASSEM_DATA_COMMAND);
+      if (new_layout != cur_layout)
 	{
-	  if (new_layout != cur_layout)
-	    {
-	      show_layout (new_layout);
+	  show_layout (new_layout);
 
-	      /* Now determine where focus should be.  */
-	      if (win_with_focus != TUI_CMD_WIN)
+	  /* Now determine where focus should be.  */
+	  if (win_with_focus != TUI_CMD_WIN)
+	    {
+	      switch (new_layout)
 		{
-		  switch (new_layout)
-		    {
-		    case SRC_COMMAND:
-		      tui_set_win_focus_to (TUI_SRC_WIN);
-		      layout_def->display_mode = SRC_WIN;
-		      layout_def->split = FALSE;
-		      break;
-		    case DISASSEM_COMMAND:
-		      /* The previous layout was not showing code.
-		         This can happen if there is no source
-		         available:
-
-		         1. if the source file is in another dir OR
-		         2. if target was compiled without -g
-		         We still want to show the assembly though!  */
-
-		      tui_get_begin_asm_address (&gdbarch, &addr);
-		      tui_set_win_focus_to (TUI_DISASM_WIN);
-		      layout_def->display_mode = DISASSEM_WIN;
-		      layout_def->split = FALSE;
-		      break;
-		    case SRC_DISASSEM_COMMAND:
-		      /* The previous layout was not showing code.
-		         This can happen if there is no source
-		         available:
-
-		         1. if the source file is in another dir OR
-		         2. if target was compiled without -g
-		         We still want to show the assembly though!  */
-
-		      tui_get_begin_asm_address (&gdbarch, &addr);
-		      if (win_with_focus == TUI_SRC_WIN)
-			tui_set_win_focus_to (TUI_SRC_WIN);
-		      else
-			tui_set_win_focus_to (TUI_DISASM_WIN);
-		      layout_def->split = TRUE;
-		      break;
-		    case SRC_DATA_COMMAND:
-		      if (win_with_focus != TUI_DATA_WIN)
-			tui_set_win_focus_to (TUI_SRC_WIN);
-		      else
-			tui_set_win_focus_to (TUI_DATA_WIN);
-		      layout_def->display_mode = SRC_WIN;
-		      layout_def->split = FALSE;
-		      break;
-		    case DISASSEM_DATA_COMMAND:
-		      /* The previous layout was not showing code.
-		         This can happen if there is no source
-		         available:
-
-			 1. if the source file is in another dir OR
-		         2. if target was compiled without -g
-		         We still want to show the assembly though!  */
-
-		      tui_get_begin_asm_address (&gdbarch, &addr);
-		      if (win_with_focus != TUI_DATA_WIN)
-			tui_set_win_focus_to (TUI_DISASM_WIN);
-		      else
-			tui_set_win_focus_to (TUI_DATA_WIN);
-		      layout_def->display_mode = DISASSEM_WIN;
-		      layout_def->split = FALSE;
-		      break;
-		    default:
-		      break;
-		    }
+		case SRC_COMMAND:
+		  tui_set_win_focus_to (TUI_SRC_WIN);
+		  layout_def->display_mode = SRC_WIN;
+		  layout_def->split = FALSE;
+		  break;
+		case DISASSEM_COMMAND:
+		  /* The previous layout was not showing code.
+		     This can happen if there is no source
+		     available:
+
+		     1. if the source file is in another dir OR
+		     2. if target was compiled without -g
+		     We still want to show the assembly though!  */
+
+		  tui_get_begin_asm_address (&gdbarch, &addr);
+		  tui_set_win_focus_to (TUI_DISASM_WIN);
+		  layout_def->display_mode = DISASSEM_WIN;
+		  layout_def->split = FALSE;
+		  break;
+		case SRC_DISASSEM_COMMAND:
+		  /* The previous layout was not showing code.
+		     This can happen if there is no source
+		     available:
+
+		     1. if the source file is in another dir OR
+		     2. if target was compiled without -g
+		     We still want to show the assembly though!  */
+
+		  tui_get_begin_asm_address (&gdbarch, &addr);
+		  if (win_with_focus == TUI_SRC_WIN)
+		    tui_set_win_focus_to (TUI_SRC_WIN);
+		  else
+		    tui_set_win_focus_to (TUI_DISASM_WIN);
+		  layout_def->split = TRUE;
+		  break;
+		case SRC_DATA_COMMAND:
+		  if (win_with_focus != TUI_DATA_WIN)
+		    tui_set_win_focus_to (TUI_SRC_WIN);
+		  else
+		    tui_set_win_focus_to (TUI_DATA_WIN);
+		  layout_def->display_mode = SRC_WIN;
+		  layout_def->split = FALSE;
+		  break;
+		case DISASSEM_DATA_COMMAND:
+		  /* The previous layout was not showing code.
+		     This can happen if there is no source
+		     available:
+
+		     1. if the source file is in another dir OR
+		     2. if target was compiled without -g
+		     We still want to show the assembly though!  */
+
+		  tui_get_begin_asm_address (&gdbarch, &addr);
+		  if (win_with_focus != TUI_DATA_WIN)
+		    tui_set_win_focus_to (TUI_DISASM_WIN);
+		  else
+		    tui_set_win_focus_to (TUI_DATA_WIN);
+		  layout_def->display_mode = DISASSEM_WIN;
+		  layout_def->split = FALSE;
+		  break;
+		default:
+		  break;
 		}
-	      /*
-	       * Now update the window content.
-	       */
-	      if (!regs_populate 
-		  && (new_layout == SRC_DATA_COMMAND 
-		      || new_layout == DISASSEM_DATA_COMMAND))
-		tui_display_all_data ();
-
-	      tui_update_source_windows_with_addr (gdbarch, addr);
 	    }
+	  /*
+	   * Now update the window content.
+	   */
+	  if (!regs_populate
+	      && (new_layout == SRC_DATA_COMMAND
+		  || new_layout == DISASSEM_DATA_COMMAND))
+	    tui_display_all_data ();
+
+	  tui_update_source_windows_with_addr (gdbarch, addr);
+
 	  if (regs_populate)
 	    {
-              tui_show_registers (TUI_DATA_WIN->detail.data_display_info.current_group);
+	      struct reggroup *group =
+		TUI_DATA_WIN->detail.data_display_info.current_group;
+	      tui_show_registers (group);
 	    }
 	}
     }
@@ -370,7 +350,6 @@ tui_default_win_viewport_height (enum tui_win_type type,
   return h;
 }
 
-
 /* Function to initialize gdb commands, for tui window layout
    manipulation.  */
 
@@ -401,10 +380,10 @@ Layout names are:\n\
 **************************/
 
 
-/* Function to set the layout to SRC, ASM, SPLIT, NEXT, PREV, DATA,
-   REGS, $REGS, $GREGS, $FREGS, $SREGS.  */
+/* Function to set the layout to SRC, ASM, SPLIT, NEXT, PREV, DATA, or
+   REGS. */
 enum tui_status
-tui_set_layout_for_display_command (const char *layout_name)
+tui_set_layout_by_name (const char *layout_name)
 {
   enum tui_status status = TUI_SUCCESS;
 
@@ -413,7 +392,6 @@ tui_set_layout_for_display_command (const char *layout_name)
       int i;
       char *buf_ptr;
       enum tui_layout_type new_layout = UNDEFINED_LAYOUT;
-      enum tui_register_display_type dpy_type = TUI_UNDEFINED_REGS;
       enum tui_layout_type cur_layout = tui_current_layout ();
 
       buf_ptr = (char *) xstrdup (layout_name);
@@ -421,8 +399,7 @@ tui_set_layout_for_display_command (const char *layout_name)
 	buf_ptr[i] = toupper (buf_ptr[i]);
 
       /* First check for ambiguous input.  */
-      if (strlen (buf_ptr) <= 1 
-	  && (*buf_ptr == 'S' || *buf_ptr == '$'))
+      if (strlen (buf_ptr) <= 1 && *buf_ptr == 'S')
 	{
 	  warning (_("Ambiguous command input."));
 	  status = TUI_FAILURE;
@@ -435,59 +412,13 @@ tui_set_layout_for_display_command (const char *layout_name)
 	    new_layout = DISASSEM_COMMAND;
 	  else if (subset_compare (buf_ptr, "SPLIT"))
 	    new_layout = SRC_DISASSEM_COMMAND;
-	  else if (subset_compare (buf_ptr, "REGS") 
-		   || subset_compare (buf_ptr, TUI_GENERAL_SPECIAL_REGS_NAME)
-		   || subset_compare (buf_ptr, TUI_GENERAL_REGS_NAME)
-		   || subset_compare (buf_ptr, TUI_FLOAT_REGS_NAME)
-		   || subset_compare (buf_ptr, TUI_SPECIAL_REGS_NAME))
+	  else if (subset_compare (buf_ptr, "REGS"))
 	    {
-	      if (cur_layout == SRC_COMMAND 
+	      if (cur_layout == SRC_COMMAND
 		  || cur_layout == SRC_DATA_COMMAND)
 		new_layout = SRC_DATA_COMMAND;
 	      else
 		new_layout = DISASSEM_DATA_COMMAND;
-
-	      /* Could ifdef out the following code. when compile with
-		 -z, there are null pointer references that cause a
-		 core dump if 'layout regs' is the first layout
-		 command issued by the user. HP has asked us to hook
-		 up this code.  - edie epstein  */
-	      if (subset_compare (buf_ptr, TUI_FLOAT_REGS_NAME))
-		{
-		  if (TUI_DATA_WIN->detail.data_display_info.regs_display_type
-		      != TUI_SFLOAT_REGS
-		      && TUI_DATA_WIN->detail.data_display_info.regs_display_type
-		      != TUI_DFLOAT_REGS)
-		    dpy_type = TUI_SFLOAT_REGS;
-		  else
-		    dpy_type =
-		      TUI_DATA_WIN->detail.data_display_info.regs_display_type;
-		}
-	      else if (subset_compare (buf_ptr,
-				      TUI_GENERAL_SPECIAL_REGS_NAME))
-		dpy_type = TUI_GENERAL_AND_SPECIAL_REGS;
-	      else if (subset_compare (buf_ptr, TUI_GENERAL_REGS_NAME))
-		dpy_type = TUI_GENERAL_REGS;
-	      else if (subset_compare (buf_ptr, TUI_SPECIAL_REGS_NAME))
-		dpy_type = TUI_SPECIAL_REGS;
-	      else if (TUI_DATA_WIN)
-		{
-		  if (TUI_DATA_WIN->detail.data_display_info.regs_display_type
-		      != TUI_UNDEFINED_REGS)
-		    dpy_type
-		      = TUI_DATA_WIN->detail.data_display_info.regs_display_type;
-		  else
-		    dpy_type = TUI_GENERAL_REGS;
-		}
-
-	      /* End of potential ifdef.
-	       */
-
-	      /* If ifdefed out code above, then assume that the user
-		 wishes to display the general purpose registers .
-	      */
-
-	      /* dpy_type = TUI_GENERAL_REGS; */
 	    }
 	  else if (subset_compare (buf_ptr, "NEXT"))
 	    new_layout = next_layout ();
@@ -496,7 +427,7 @@ tui_set_layout_for_display_command (const char *layout_name)
 	  else
 	    status = TUI_FAILURE;
 
-	  tui_set_layout (new_layout, dpy_type);
+	  tui_set_layout (new_layout);
 	}
       xfree (buf_ptr);
     }
@@ -549,7 +480,7 @@ tui_layout_command (char *arg, int from_tty)
   tui_enable ();
 
   /* Switch to the selected layout.  */
-  if (tui_set_layout_for_display_command (arg) != TUI_SUCCESS)
+  if (tui_set_layout_by_name (arg) != TUI_SUCCESS)
     warning (_("Invalid layout specified.\n%s"), LAYOUT_USAGE);
 
 }
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index 6dedb7d..623d254 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -30,7 +30,6 @@ extern int tui_default_win_height (enum tui_win_type,
 				   enum tui_layout_type);
 extern int tui_default_win_viewport_height (enum tui_win_type,
 					    enum tui_layout_type);
-extern enum tui_status tui_set_layout (enum tui_layout_type,
-				       enum tui_register_display_type);
+extern enum tui_status tui_set_layout (enum tui_layout_type);
 
 #endif /*TUI_LAYOUT_H */
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 61918f8..903c7c6 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -139,7 +139,7 @@ tui_show_registers (struct reggroup *group)
   /* Make sure the register window is visible.  If not, select an
      appropriate layout.  */
   if (TUI_DATA_WIN == NULL || !TUI_DATA_WIN->generic.is_visible)
-    tui_set_layout_for_display_command (DATA_NAME);
+    tui_set_layout_by_name (DATA_NAME);
 
   display_info = &TUI_DATA_WIN->detail.data_display_info;
   if (group == 0)
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index b04e106..308e7ae 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -150,7 +150,6 @@ tui_rl_change_windows (int notused1, int notused2)
   if (tui_active)
     {
       enum tui_layout_type new_layout;
-      enum tui_register_display_type regs_type = TUI_UNDEFINED_REGS;
 
       new_layout = tui_current_layout ();
 
@@ -182,7 +181,7 @@ tui_rl_change_windows (int notused1, int notused2)
 	  new_layout = SRC_COMMAND;
 	  break;
 	}
-      tui_set_layout (new_layout, regs_type);
+      tui_set_layout (new_layout);
     }
   return 0;
 }
@@ -198,7 +197,6 @@ tui_rl_delete_other_windows (int notused1, int notused2)
   if (tui_active)
     {
       enum tui_layout_type new_layout;
-      enum tui_register_display_type regs_type = TUI_UNDEFINED_REGS;
 
       new_layout = tui_current_layout ();
 
@@ -217,7 +215,7 @@ tui_rl_delete_other_windows (int notused1, int notused2)
 	  new_layout = DISASSEM_COMMAND;
 	  break;
 	}
-      tui_set_layout (new_layout, regs_type);
+      tui_set_layout (new_layout);
     }
   return 0;
 }
@@ -464,7 +462,7 @@ tui_enable (void)
       def_prog_mode ();
 
       tui_show_frame_info (0);
-      tui_set_layout (SRC_COMMAND, TUI_UNDEFINED_REGS);
+      tui_set_layout (SRC_COMMAND);
       tui_set_win_focus_to (TUI_SRC_WIN);
       keypad (TUI_CMD_WIN->generic.handle, TRUE);
       wrefresh (TUI_CMD_WIN->generic.handle);
diff --git a/gdb/tui/tui.h b/gdb/tui/tui.h
index 2ce5705..59bb7c9 100644
--- a/gdb/tui/tui.h
+++ b/gdb/tui/tui.h
@@ -96,6 +96,6 @@ extern void tui_show_source (const char *fullname, int line);
 extern struct ui_out *tui_out_new (struct ui_file *stream);
 
 /* tui-layout.c */
-extern enum tui_status tui_set_layout_for_display_command (const char *);
+extern enum tui_status tui_set_layout_by_name (const char *);
 
 #endif

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

* Re: [PATCH 1/4] gdb: Remove register class specific layout names.
  2015-05-21 12:25   ` Andrew Burgess
@ 2015-05-21 13:17     ` Pedro Alves
  0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2015-05-21 13:17 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 05/21/2015 01:25 PM, Andrew Burgess wrote:
> 
> * Pedro Alves <palves@redhat.com> [2015-05-21 09:12:45 +0100]:

>> Doesn't your series effectively make this bit in display_command dead?
>> (while before it would switch on the registers layout).
> 
> Sorry for missing that use.
> 
> You're right, it's certainly dead now, though before my patch it still
> didn't do anything, passing the '$FREGS', etc names to
> tui_set_layout_for_display_command would just cause the regs display
> to pop up with the default register set. (At least, that's all I
> ever see, and I don't think anything else is possible.)

Right, that's what I saw.

> 
> Given that it's not worked for a while, I propose to just remove the
> above snippet (really having this in display seems nasty to me) and
> just move to layout being for layout adjustment, and 'tui reg' to
> change the displayed register set.

Agreed.

> 
>>                                                         (We should probably
>> rename tui_set_layout_for_display_command too.)
> 
> Done, it's now tui_set_layout_by_name.
> 
>> I had never noticed these special register layouts before either.  I'm not
>> at all adverse to removing them.  Not all expressions that start with $ are
>> registers, and probably a better idea would be to have
>> a separate "displays" window (so displays would go to that window
>> instead of the command window when the TUI is active), so that the tui
>> could neatly show watched variables/random expressions too.
> 
> Sounds like a good idea.
> 
> New patch below.

OK.

Thanks,
Pedro Alves

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

end of thread, other threads:[~2015-05-21 13:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-20 23:17 [PATCH 0/4] layout command changes Andrew Burgess
2015-05-20 23:18 ` [PATCH 2/4] gdb: Add completer for layout command Andrew Burgess
2015-05-21  0:25   ` Keith Seitz
2015-05-21  7:10     ` Andrew Burgess
2015-05-20 23:18 ` [PATCH 4/4] gdb: Add cleanup to avoid memory leak on error Andrew Burgess
2015-05-20 23:18 ` [PATCH 3/4] gdb: Don't call tui_enable too early Andrew Burgess
2015-05-20 23:18 ` [PATCH 1/4] gdb: Remove register class specific layout names Andrew Burgess
2015-05-21  8:42   ` Pedro Alves
2015-05-21 11:33     ` Andrew Burgess
2015-05-21 11:34       ` Pedro Alves
2015-05-21 12:25   ` Andrew Burgess
2015-05-21 13:17     ` Pedro Alves
2015-05-21  8:12 ` [PATCH 0/4] layout command changes Pedro Alves
2015-05-21  8:35   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).