public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCHv2 1/3] gdb/python: allow Python TUI windows to be replaced
Date: Wed, 25 Jan 2023 13:56:09 +0000	[thread overview]
Message-ID: <e339233364c78bcc9a284c9c76ad44671a1941ba.1674654912.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1674654912.git.aburgess@redhat.com>

The documentation for gdb.register_window_type says:

  "...  It's an error to try to replace one of the built-in windows,
  but other window types can be replaced. ..."

I take this to mean that if I imported a Python script like this:

  gdb.register_window_type('my_window', FactoryFunction)

Then GDB would have a new TUI window 'my_window', which could be
created by calling FactoryFunction().  If I then, in the same GDB
session imported a script which included:

  gdb.register_window_type('my_window', UpdatedFactoryFunction)

Then GDB would replace the old 'my_window' factory with my new one,
GDB would now call UpdatedFactoryFunction().

This is pretty useful in practice, as it allows users to iterate on
their window implementation within a single GDB session.

However, right now, this is not how GDB operates.  The second call to
register_window_type is basically ignored and the old window factory
is retained.

This is because in tui_register_window (tui/tui-layout.c) we use
std::unordered_map::emplace to insert the new factory function, and
emplace doesn't replace an existing element in an unordered_map.

In this commit, before the emplace call, I now search for an already
existing element, and delete any matching element from the map, the
emplace call will then add the new factory function.
---
 .../gdb.python/tui-window-factory.exp         | 73 +++++++++++++++++++
 .../gdb.python/tui-window-factory.py          | 48 ++++++++++++
 gdb/tui/tui-layout.c                          |  8 ++
 3 files changed, 129 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/tui-window-factory.exp
 create mode 100644 gdb/testsuite/gdb.python/tui-window-factory.py

diff --git a/gdb/testsuite/gdb.python/tui-window-factory.exp b/gdb/testsuite/gdb.python/tui-window-factory.exp
new file mode 100644
index 00000000000..99f9fbb1bc4
--- /dev/null
+++ b/gdb/testsuite/gdb.python/tui-window-factory.exp
@@ -0,0 +1,73 @@
+# Copyright (C) 2023 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that GDB correctly deallocates the window factory object (a)
+# when a window factory is replaced, and (b) during GDB shutdown.
+#
+# This test also ensures that when a new window is registered (via the
+# Python API) with the same name as an existing window, then the
+# previous window is replaced.
+
+load_lib gdb-python.exp
+
+tuiterm_env
+
+clean_restart
+
+require allow_tui_tests allow_python_tests
+
+set pyfile [gdb_remote_download host \
+		${srcdir}/${subdir}/${gdb_test_file_name}.py]
+
+Term::clean_restart 24 80
+Term::prepare_for_tui
+
+gdb_test "source ${pyfile}" "Python script imported" \
+    "import python scripts"
+
+gdb_test "python register_window_factory('msg_1')" \
+    "Entering TestWindowFactory\\.__init__: msg_1"
+
+gdb_test "python register_window_factory('msg_2')" \
+    [multi_line \
+	 "Entering TestWindowFactory\\.__init__: msg_2" \
+	 "Entering TestWindowFactory\\.__del__: msg_1"]
+
+gdb_test_no_output "tui new-layout test test_window 1 cmd 1 status 1"
+
+# Load the custom window layout and ensure that the correct window
+# factory was used.
+with_test_prefix "msg_2" {
+    Term::command_no_prompt_prefix "layout test"
+    Term::check_box_contents "check test_window box" 0 0 80 15 \
+	"TestWindow \\(msg_2\\)"
+}
+
+# Replace the existing window factory with a new one, then switch
+# layouts so that GDB recreates the window, and check that the new
+# window factory was used.
+with_test_prefix "msg_3" {
+    Term::command "python register_window_factory('msg_3')"
+    Term::check_region_contents "check for python output" \
+	0 18 80 2 \
+	[multi_line \
+	     "Entering TestWindowFactory.__init__: msg_3\\s+" \
+	     "Entering TestWindowFactory.__del__: msg_2"]
+    Term::command "layout src"
+    Term::command "layout test"
+
+    Term::check_box_contents "check test_window box" 0 0 80 15 \
+	"TestWindow \\(msg_3\\)"
+}
diff --git a/gdb/testsuite/gdb.python/tui-window-factory.py b/gdb/testsuite/gdb.python/tui-window-factory.py
new file mode 100644
index 00000000000..d1254e7e3a0
--- /dev/null
+++ b/gdb/testsuite/gdb.python/tui-window-factory.py
@@ -0,0 +1,48 @@
+# Copyright (C) 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+class TestWindow:
+    def __init__(self, tui_win, msg):
+        self.msg = msg
+        self.tui_win = tui_win
+        print("Entering TestWindow.__init__: %s" % self.msg)
+
+    def render(self):
+        self.tui_win.erase()
+        self.tui_win.write("TestWindow (%s)" % self.msg)
+
+    def __del__(self):
+        print("Entering TestWindow.__del__: %s" % self.msg)
+
+
+class TestWindowFactory:
+    def __init__(self, msg):
+        self.msg = msg
+        print("Entering TestWindowFactory.__init__: %s" % self.msg)
+
+    def __call__(self, tui_win):
+        print("Entering TestWindowFactory.__call__: %s" % self.msg)
+        return TestWindow(tui_win, self.msg)
+
+    def __del__(self):
+        print("Entering TestWindowFactory.__del__: %s" % self.msg)
+
+
+def register_window_factory(msg):
+    gdb.register_window_type("test_window", TestWindowFactory(msg))
+
+
+print("Python script imported")
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 27abee02087..b895e00a80d 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -418,6 +418,14 @@ tui_register_window (const char *name, window_factory &&factory)
   if (!ISALPHA (name_copy[0]))
     error (_("window name must start with a letter, not '%c'"), name_copy[0]);
 
+  /* We already check above for all the builtin window names.  If we get
+     this far then NAME must be a user defined window.  Remove any existing
+     factory and replace it with this new version.  */
+
+  auto iter = known_window_types->find (name);
+  if (iter != known_window_types->end ())
+    known_window_types->erase (iter);
+
   known_window_types->emplace (std::move (name_copy),
 			       std::move (factory));
 }
-- 
2.25.4


  reply	other threads:[~2023-01-25 13:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-12 19:19 [PATCH 0/3] Python/TUI Window Creation / Destruction Fixes Andrew Burgess
2023-01-12 19:19 ` [PATCH 1/3] gdb/python: allow Python TUI windows to be replaced Andrew Burgess
2023-01-12 19:19 ` [PATCH 2/3] gdb/python: deallocate tui window factories at Python shut down Andrew Burgess
2023-01-12 19:19 ` [PATCH 3/3] gdb/tui: don't leak the known_window_types map Andrew Burgess
2023-01-25 13:56 ` [PATCHv2 0/3] Python/TUI Window Creation / Destruction Fixes Andrew Burgess
2023-01-25 13:56   ` Andrew Burgess [this message]
2023-01-25 13:56   ` [PATCHv2 2/3] gdb/python: deallocate tui window factories at Python shut down Andrew Burgess
2023-01-25 13:56   ` [PATCHv2 3/3] gdb/tui: don't leak the known_window_types map Andrew Burgess
2023-01-27 16:34   ` [PATCHv3 0/3] Python/TUI Window Creation / Destruction Fixes Andrew Burgess
2023-01-27 16:34     ` [PATCHv3 1/3] gdb/python: allow Python TUI windows to be replaced Andrew Burgess
2023-01-27 16:34     ` [PATCHv3 2/3] gdb/python: deallocate tui window factories at Python shut down Andrew Burgess
2023-01-27 16:34     ` [PATCHv3 3/3] gdb/tui: don't leak the known_window_types map Andrew Burgess
2023-02-13 12:32     ` [PATCHv3 0/3] Python/TUI Window Creation / Destruction Fixes Andrew Burgess
2023-02-13 14:11       ` Tom Tromey
2023-02-13 14:52         ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e339233364c78bcc9a284c9c76ad44671a1941ba.1674654912.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).