aboutsummaryrefslogtreecommitdiff
path: root/gdb
diff options
context:
space:
mode:
authorAndrew Burgess <andrew.burgess@embecosm.com>2021-01-25 15:46:58 +0000
committerAndrew Burgess <andrew.burgess@embecosm.com>2021-02-08 11:18:33 +0000
commit1cf2399651ef3fe1350ad8276cf00d16ddeb9960 (patch)
treeef7670ef713ccdc3f2013d9477a0c4882032c5d7 /gdb
parenta53a265752ef4b911d175aea62e082e54e717497 (diff)
downloadgdb-1cf2399651ef3fe1350ad8276cf00d16ddeb9960.zip
gdb-1cf2399651ef3fe1350ad8276cf00d16ddeb9960.tar.gz
gdb-1cf2399651ef3fe1350ad8276cf00d16ddeb9960.tar.bz2
gdb/tui: don't add windows to global list from tui_layout:window::apply
This commit was inspired by this mailing list patch: https://sourceware.org/pipermail/gdb-patches/2021-January/174713.html Currently, calling tui_layout_window::apply will add the window from the layout object to the global tui_windows list. Unfortunately, when the user runs the 'winheight' command, this calls tui_adjust_window_height, which calls the tui_layout_base::adjust_size function, which can then call tui_layout_base::apply. The consequence of this is that when the user does 'winheight' duplicate copies of a window can be added to the global tui_windows list. The original patch fixed this by changing the apply function to only update the global list some of the time. This patch takes a different approach. The apply function no longer updates the global tui_windows list. Instead a new virtual function is added to tui_layout_base which is used to gather all the currently applied windows into a vector. Finally tui_apply_current_layout is updated to make use of this new function to update the tui_windows list. The benefits I see in this approach are, (a) the apply function now no longer touches global state, this solves the immediate problem, and (b) now that tui_windows is updated directly in the function tui_apply_current_layout, we can drop the saved_tui_windows global. gdb/ChangeLog: * tui-layout.c (saved_tui_windows): Delete. (tui_apply_current_layout): Don't make use of saved_tui_windows, call new get_windows member function instead. (tui_get_window_by_name): Check in tui_windows. (tui_layout_window::apply): Don't add to tui_windows. * tui-layout.h (tui_layout_base::get_windows): New member function. (tui_layout_window::get_windows): Likewise. (tui_layout_split::get_windows): Likewise. gdb/testsuite/ChangeLog: * gdb.tui/winheight.exp: Add more tests.
Diffstat (limited to 'gdb')
-rw-r--r--gdb/ChangeLog11
-rw-r--r--gdb/testsuite/ChangeLog4
-rw-r--r--gdb/testsuite/gdb.tui/winheight.exp14
-rw-r--r--gdb/tui/tui-layout.c26
-rw-r--r--gdb/tui/tui-layout.h16
5 files changed, 56 insertions, 15 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3b9615d..d450be6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,16 @@
2021-02-08 Andrew Burgess <andrew.burgess@embecosm.com>
+ * tui-layout.c (saved_tui_windows): Delete.
+ (tui_apply_current_layout): Don't make use of saved_tui_windows,
+ call new get_windows member function instead.
+ (tui_get_window_by_name): Check in tui_windows.
+ (tui_layout_window::apply): Don't add to tui_windows.
+ * tui-layout.h (tui_layout_base::get_windows): New member function.
+ (tui_layout_window::get_windows): Likewise.
+ (tui_layout_split::get_windows): Likewise.
+
+2021-02-08 Andrew Burgess <andrew.burgess@embecosm.com>
+
* tui/tui-layout.c (tui_apply_current_layout): Restore the delete
of the window objects.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 8fb69c0..de68598 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
2021-02-08 Andrew Burgess <andrew.burgess@embecosm.com>
+ * gdb.tui/winheight.exp: Add more tests.
+
+2021-02-08 Andrew Burgess <andrew.burgess@embecosm.com>
+
* gdb.python/py-framefilter.exp: Update expected results.
* gdb.python/python.exp: Update expected results.
diff --git a/gdb/testsuite/gdb.tui/winheight.exp b/gdb/testsuite/gdb.tui/winheight.exp
index 38fb29c..04de35d 100644
--- a/gdb/testsuite/gdb.tui/winheight.exp
+++ b/gdb/testsuite/gdb.tui/winheight.exp
@@ -36,3 +36,17 @@ Term::check_box "smaller source box" 0 0 80 10
Term::command "winheight cmd -5"
Term::check_box "larger source box" 0 0 80 15
+
+Term::command "winheight src -5"
+Term::check_box "smaller source box again" 0 0 80 10
+
+Term::command "winheight src +5"
+Term::check_box "larger source box again" 0 0 80 15
+
+# At one point we had a bug where adjusting the winheight would result
+# in GDB keeping hold of duplicate window pointers, which it might
+# then try to delete when the layout was changed. Running this test
+# under valgrind would expose that bug.
+Term::command "layout asm"
+Term::check_box "check for asm window" 0 0 80 15
+
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index f01e2f9..f08d62d 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -64,11 +64,6 @@ static tui_layout_split *asm_regs_layout;
/* See tui-data.h. */
std::vector<tui_win_info *> tui_windows;
-/* When applying a layout, this is the list of all windows that were
- in the previous layout. This is used to re-use windows when
- changing a layout. */
-static std::vector<tui_win_info *> saved_tui_windows;
-
/* See tui-layout.h. */
void
@@ -79,10 +74,7 @@ tui_apply_current_layout ()
extract_display_start_addr (&gdbarch, &addr);
- saved_tui_windows = std::move (tui_windows);
- tui_windows.clear ();
-
- for (tui_win_info *win_info : saved_tui_windows)
+ for (tui_win_info *win_info : tui_windows)
win_info->make_visible (false);
applied_layout->apply (0, 0, tui_term_width (), tui_term_height ());
@@ -96,23 +88,28 @@ tui_apply_current_layout ()
/* This should always be made visible by a layout. */
gdb_assert (TUI_CMD_WIN->is_visible ());
+ /* Get the new list of currently visible windows. */
+ std::vector<tui_win_info *> new_tui_windows;
+ applied_layout->get_windows (&new_tui_windows);
+
/* Now delete any window that was not re-applied. */
tui_win_info *focus = tui_win_with_focus ();
- for (tui_win_info *win_info : saved_tui_windows)
+ for (tui_win_info *win_info : tui_windows)
{
if (!win_info->is_visible ())
{
if (focus == win_info)
- tui_set_win_focus_to (tui_windows[0]);
+ tui_set_win_focus_to (new_tui_windows[0]);
delete win_info;
}
}
+ /* Replace the global list of active windows. */
+ tui_windows = std::move (new_tui_windows);
+
if (gdbarch == nullptr && TUI_DISASM_WIN != nullptr)
tui_get_begin_asm_address (&gdbarch, &addr);
tui_update_source_windows_with_addr (gdbarch, addr);
-
- saved_tui_windows.clear ();
}
/* See tui-layout. */
@@ -343,7 +340,7 @@ static std::unordered_map<std::string, window_factory> *known_window_types;
static tui_win_info *
tui_get_window_by_name (const std::string &name)
{
- for (tui_win_info *window : saved_tui_windows)
+ for (tui_win_info *window : tui_windows)
if (name == window->name ())
return window;
@@ -415,7 +412,6 @@ tui_layout_window::apply (int x_, int y_, int width_, int height_)
height = height_;
gdb_assert (m_window != nullptr);
m_window->resize (height, width, x, y);
- tui_windows.push_back (m_window);
}
/* See tui-layout.h. */
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index 193f42d..f89166e 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -91,6 +91,9 @@ public:
depth of this layout in the hierarchy (zero-based). */
virtual void specification (ui_file *output, int depth) = 0;
+ /* Add all windows to the WINDOWS vector. */
+ virtual void get_windows (std::vector<tui_win_info *> *windows) = 0;
+
/* The most recent space allocation. */
int x = 0;
int y = 0;
@@ -141,6 +144,12 @@ public:
void specification (ui_file *output, int depth) override;
+ /* See tui_layout_base::get_windows. */
+ void get_windows (std::vector<tui_win_info *> *windows) override
+ {
+ windows->push_back (m_window);
+ }
+
protected:
void get_sizes (bool height, int *min_value, int *max_value) override;
@@ -195,6 +204,13 @@ public:
void specification (ui_file *output, int depth) override;
+ /* See tui_layout_base::get_windows. */
+ void get_windows (std::vector<tui_win_info *> *windows) override
+ {
+ for (auto &item : m_splits)
+ item.layout->get_windows (windows);
+ }
+
protected:
void get_sizes (bool height, int *min_value, int *max_value) override;