[1/2] Fix C-x 1 from gdb prompt

Message ID 20200606201301.19145-2-tom@tromey.com
State New
Headers show
Series
  • Fix two TUI crashes
Related show

Commit Message

Tom Tromey June 6, 2020, 8:13 p.m.
Pedro pointed out on irc that C-x 1 from the gdb prompt will cause a
crash.  This happened because of a bug in remove_windows -- it would
always remove all the windows from the layout.  This patch fixes this
bug, and also arranges to have C-x 1 preserve the status window.

gdb/ChangeLog
2020-06-06  Tom Tromey  <tom@tromey.com>

	* tui/tui-layout.c (tui_layout_split::remove_windows): Fix logic.
	Also preserve the status window.
---
 gdb/ChangeLog        |  5 +++++
 gdb/tui/tui-layout.c | 11 ++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

-- 
2.17.2

Comments

Simon Marchi via Gdb-patches June 8, 2020, 5:57 p.m. | #1
On Sat, Jun 6, 2020 at 3:13 PM Tom Tromey <tom@tromey.com> wrote:
> +      else if (strcmp (this_name, name) == 0

> +              || strcmp (this_name, "cmd") == 0

> +              || strcmp (this_name, "status") == 0)


Maybe those strings should be constants somewhere?

Christian
Tom Tromey June 12, 2020, 12:26 a.m. | #2
>>>>> "Christian" == Christian Biesinger <cbiesinger@google.com> writes:


Christian> On Sat, Jun 6, 2020 at 3:13 PM Tom Tromey <tom@tromey.com> wrote:
>> +      else if (strcmp (this_name, name) == 0

>> +              || strcmp (this_name, "cmd") == 0

>> +              || strcmp (this_name, "status") == 0)


Christian> Maybe those strings should be constants somewhere?

Most of them were constants in tui-data.h already.
I thought I'd tack this patch on to the end of the series.

Tom

commit 9aaf1b1b02745ee803dcfa2b7bf91add2cf0a6ea
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Jun 11 18:19:12 2020 -0600

    Use macros for TUI window names
    
    Christian pointed out that tui-layout.c hard-codes various window
    names.  This patch changes the code to use the macros from tui-data.h
    instead.  For each window, I searched for uses of the name; but I only
    found any in tui-layout.c.  This also adds a new macro to account for
    the "status" window.
    
    gdb/ChangeLog
    2020-06-11  Tom Tromey  <tom@tromey.com>
    
            * tui/tui-data.h (STATUS_NAME): New macro.
            * tui/tui-layout.c (tui_remove_some_windows)
            (initialize_known_windows, tui_register_window)
            (tui_layout_split::remove_windows, initialize_layouts)
            (tui_new_layout_command): Don't use hard-coded window names.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cd6ff0b8fe8..f674fef7e16 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2020-06-11  Tom Tromey  <tom@tromey.com>
+
+	* tui/tui-data.h (STATUS_NAME): New macro.
+	* tui/tui-layout.c (tui_remove_some_windows)
+	(initialize_known_windows, tui_register_window)
+	(tui_layout_split::remove_windows, initialize_layouts)
+	(tui_new_layout_command): Don't use hard-coded window names.
+
 2020-06-06  Tom Tromey  <tom@tromey.com>
 
 	PR tui/25348:
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 2dc73b963da..abe77272291 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -130,6 +130,7 @@ struct tui_gen_win_info
 #define CMD_NAME                "cmd"
 #define DATA_NAME               "regs"
 #define DISASSEM_NAME           "asm"
+#define STATUS_NAME		"status"
 #define MIN_WIN_HEIGHT          3
 #define MIN_CMD_WIN_HEIGHT      3
 
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index b87d21eb6de..8164b346370 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -255,7 +255,7 @@ tui_remove_some_windows ()
 {
   tui_win_info *focus = tui_win_with_focus ();
 
-  if (strcmp (focus->name (), "cmd") == 0)
+  if (strcmp (focus->name (), CMD_NAME) == 0)
     {
       /* Try leaving the source or disassembly window.  If neither
 	 exists, just do nothing.  */
@@ -373,18 +373,18 @@ initialize_known_windows ()
 {
   known_window_types = new std::unordered_map<std::string, window_factory>;
 
-  known_window_types->emplace ("src",
+  known_window_types->emplace (SRC_NAME,
 			       make_standard_window<SRC_WIN,
 						    tui_source_window>);
-  known_window_types->emplace ("cmd",
+  known_window_types->emplace (CMD_NAME,
 			       make_standard_window<CMD_WIN, tui_cmd_window>);
-  known_window_types->emplace ("regs",
+  known_window_types->emplace (DATA_NAME,
 			       make_standard_window<DATA_WIN,
 						    tui_data_window>);
-  known_window_types->emplace ("asm",
+  known_window_types->emplace (DISASSEM_NAME,
 			       make_standard_window<DISASSEM_WIN,
 						    tui_disasm_window>);
-  known_window_types->emplace ("status", get_locator_window);
+  known_window_types->emplace (STATUS_NAME, get_locator_window);
 }
 
 /* See tui-layout.h.  */
@@ -394,8 +394,8 @@ tui_register_window (const char *name, window_factory &&factory)
 {
   std::string name_copy = name;
 
-  if (name_copy == "src" || name_copy == "cmd" || name_copy == "regs"
-      || name_copy == "asm" || name_copy == "status")
+  if (name_copy == SRC_NAME || name_copy == CMD_NAME || name_copy == DATA_NAME
+      || name_copy == DISASSEM_NAME || name_copy == STATUS_NAME)
     error (_("Window type \"%s\" is built-in"), name);
 
   known_window_types->emplace (std::move (name_copy),
@@ -791,8 +791,8 @@ tui_layout_split::remove_windows (const char *name)
       if (this_name == nullptr)
 	m_splits[i].layout->remove_windows (name);
       else if (strcmp (this_name, name) == 0
-	       || strcmp (this_name, "cmd") == 0
-	       || strcmp (this_name, "status") == 0)
+	       || strcmp (this_name, CMD_NAME) == 0
+	       || strcmp (this_name, STATUS_NAME) == 0)
 	{
 	  /* Keep.  */
 	}
@@ -888,37 +888,37 @@ initialize_layouts ()
   tui_layout_split *layout;
 
   layout = new tui_layout_split ();
-  layout->add_window ("src", 2);
-  layout->add_window ("status", 0);
-  layout->add_window ("cmd", 1);
-  add_layout_command ("src", layout);
+  layout->add_window (SRC_NAME, 2);
+  layout->add_window (STATUS_NAME, 0);
+  layout->add_window (CMD_NAME, 1);
+  add_layout_command (SRC_NAME, layout);
 
   layout = new tui_layout_split ();
-  layout->add_window ("asm", 2);
-  layout->add_window ("status", 0);
-  layout->add_window ("cmd", 1);
-  add_layout_command ("asm", layout);
+  layout->add_window (DISASSEM_NAME, 2);
+  layout->add_window (STATUS_NAME, 0);
+  layout->add_window (CMD_NAME, 1);
+  add_layout_command (DISASSEM_NAME, layout);
 
   layout = new tui_layout_split ();
-  layout->add_window ("src", 1);
-  layout->add_window ("asm", 1);
-  layout->add_window ("status", 0);
-  layout->add_window ("cmd", 1);
+  layout->add_window (SRC_NAME, 1);
+  layout->add_window (DISASSEM_NAME, 1);
+  layout->add_window (STATUS_NAME, 0);
+  layout->add_window (CMD_NAME, 1);
   add_layout_command ("split", layout);
 
   layout = new tui_layout_split ();
-  layout->add_window ("regs", 1);
-  layout->add_window ("src", 1);
-  layout->add_window ("status", 0);
-  layout->add_window ("cmd", 1);
+  layout->add_window (DATA_NAME, 1);
+  layout->add_window (SRC_NAME, 1);
+  layout->add_window (STATUS_NAME, 0);
+  layout->add_window (CMD_NAME, 1);
   layouts.emplace_back (layout);
   src_regs_layout = layout;
 
   layout = new tui_layout_split ();
-  layout->add_window ("regs", 1);
-  layout->add_window ("asm", 1);
-  layout->add_window ("status", 0);
-  layout->add_window ("cmd", 1);
+  layout->add_window (DATA_NAME, 1);
+  layout->add_window (DISASSEM_NAME, 1);
+  layout->add_window (STATUS_NAME, 0);
+  layout->add_window (CMD_NAME, 1);
   layouts.emplace_back (layout);
   asm_regs_layout = layout;
 }
@@ -1010,8 +1010,8 @@ tui_new_layout_command (const char *spec, int from_tty)
     error (_("Missing '}' in layout specification"));
   if (seen_windows.empty ())
     error (_("New layout does not contain any windows"));
-  if (seen_windows.find ("cmd") == seen_windows.end ())
-    error (_("New layout does not contain the \"cmd\" window"));
+  if (seen_windows.find (CMD_NAME) == seen_windows.end ())
+    error (_("New layout does not contain the \"" CMD_NAME "\" window"));
 
   gdb::unique_xmalloc_ptr<char> cmd_name
     = make_unique_xstrdup (new_name.c_str ());

Patch

diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 491ce275acb..b87d21eb6de 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -790,13 +790,14 @@  tui_layout_split::remove_windows (const char *name)
       const char *this_name = m_splits[i].layout->get_name ();
       if (this_name == nullptr)
 	m_splits[i].layout->remove_windows (name);
+      else if (strcmp (this_name, name) == 0
+	       || strcmp (this_name, "cmd") == 0
+	       || strcmp (this_name, "status") == 0)
+	{
+	  /* Keep.  */
+	}
       else
 	{
-	  if (strcmp (this_name, name) == 0
-	      || strcmp (this_name, "cmd") == 0)
-	    {
-	      /* Keep.  */
-	    }
 	  m_splits.erase (m_splits.begin () + i);
 	  --i;
 	}