public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Replace deprecated_target_wait_hook by observers
@ 2021-08-26 14:40 Patrick Monnerat
  2021-09-04 11:58 ` [PING] " Patrick Monnerat
  2021-09-07 15:08 ` [PATCH] " Simon Marchi
  0 siblings, 2 replies; 9+ messages in thread
From: Patrick Monnerat @ 2021-08-26 14:40 UTC (permalink / raw)
  To: gdb-patches

Commit b60cea7 (Make target_wait options use enum flags) broke
deprecated_target_wait_hook usage: there's a commit comment telling
this hook has not been converted.

Rather than trying to mend it, this patch replaces the hook by two
target_wait observers:

target_pre_wait (ptid_t ptid)
target_post_wait (ptid_t event_ptid)

Upon target_wait entry, target_pre_wait is notified with the ptid
passed to target_wait. Upon exit, target_post_wait is notified with
the event ptid returned by target_wait. Should an exception occur,
event_ptid is null_ptid.

This change benefits to Insight (out-of-tree): there's no real use of the
late hook in gdb itself.
---
 gdb/infrun.c     | 15 +++------------
 gdb/infrun.h     |  5 ++---
 gdb/interps.c    |  1 -
 gdb/observable.c |  2 ++
 gdb/observable.h |  6 ++++++
 gdb/target.c     | 37 ++++++++++++++++++++++++++++++++++++-
 gdb/top.c        |  7 -------
 7 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 5ee650fa464..695e3b0a586 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -366,7 +366,7 @@ show_stop_on_solib_events (struct ui_file *file, int from_tty,
 static bool stop_print_frame;
 
 /* This is a cached copy of the target/ptid/waitstatus of the last
-   event returned by target_wait()/deprecated_target_wait_hook().
+   event returned by target_wait().
    This information is returned by get_last_target_status().  */
 static process_stratum_target *target_last_proc_target;
 static ptid_t target_last_wait_ptid;
@@ -3478,7 +3478,6 @@ static ptid_t
 do_target_wait_1 (inferior *inf, ptid_t ptid,
 		  target_waitstatus *status, target_wait_flags options)
 {
-  ptid_t event_ptid;
   struct thread_info *tp;
 
   /* We know that we are looking for an event in the target of inferior
@@ -3594,12 +3593,7 @@ do_target_wait_1 (inferior *inf, ptid_t ptid,
   if (!target_can_async_p ())
     options &= ~TARGET_WNOHANG;
 
-  if (deprecated_target_wait_hook)
-    event_ptid = deprecated_target_wait_hook (ptid, status, options);
-  else
-    event_ptid = target_wait (ptid, status, options);
-
-  return event_ptid;
+  return target_wait (ptid, status, options);
 }
 
 /* Wrapper for target_wait that first checks whether threads have
@@ -4558,10 +4552,7 @@ poll_one_curr_target (struct target_waitstatus *ws)
      don't get any event.  */
   target_dcache_invalidate ();
 
-  if (deprecated_target_wait_hook)
-    event_ptid = deprecated_target_wait_hook (minus_one_ptid, ws, TARGET_WNOHANG);
-  else
-    event_ptid = target_wait (minus_one_ptid, ws, TARGET_WNOHANG);
+  event_ptid = target_wait (minus_one_ptid, ws, TARGET_WNOHANG);
 
   if (debug_infrun)
     print_target_wait_results (minus_one_ptid, event_ptid, ws);
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 5a577365f94..a14e20b5f94 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -124,9 +124,8 @@ extern process_stratum_target *user_visible_resume_target (ptid_t resume_ptid);
 extern int normal_stop (void);
 
 /* Return the cached copy of the last target/ptid/waitstatus returned
-   by target_wait()/deprecated_target_wait_hook().  The data is
-   actually cached by handle_inferior_event(), which gets called
-   immediately after target_wait()/deprecated_target_wait_hook().  */
+   by target_wait().  The data is actually cached by handle_inferior_event(),
+   which gets called immediately after target_wait().  */
 extern void get_last_target_status (process_stratum_target **target,
 				    ptid_t *ptid,
 				    struct target_waitstatus *status);
diff --git a/gdb/interps.c b/gdb/interps.c
index ec19822b571..55bd10467c3 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -356,7 +356,6 @@ clear_interpreter_hooks (void)
   deprecated_readline_hook = 0;
   deprecated_readline_end_hook = 0;
   deprecated_context_hook = 0;
-  deprecated_target_wait_hook = 0;
   deprecated_call_command_hook = 0;
   deprecated_error_begin_hook = 0;
 }
diff --git a/gdb/observable.c b/gdb/observable.c
index 51f5edb0a1f..5724b712604 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -77,6 +77,8 @@ DEFINE_OBSERVABLE (register_changed);
 DEFINE_OBSERVABLE (user_selected_context_changed);
 DEFINE_OBSERVABLE (source_styling_changed);
 DEFINE_OBSERVABLE (current_source_symtab_and_line_changed);
+DEFINE_OBSERVABLE (target_pre_wait);
+DEFINE_OBSERVABLE (target_post_wait);
 
 } /* namespace observers */
 } /* namespace gdb */
diff --git a/gdb/observable.h b/gdb/observable.h
index 915770ff363..652fb3df43f 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -251,6 +251,12 @@ extern observable<> source_styling_changed;
 
 extern observable<> current_source_symtab_and_line_changed;
 
+/* About to enter target_wait (). */
+extern observable <ptid_t /* ptid */> target_pre_wait;
+
+/* About to leave target_wait (). */
+extern observable <ptid_t /* event_ptid */> target_post_wait;
+
 } /* namespace observers */
 
 } /* namespace gdb */
diff --git a/gdb/target.c b/gdb/target.c
index ae2d659583e..22e58484a18 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -26,6 +26,7 @@
 #include "symtab.h"
 #include "inferior.h"
 #include "infrun.h"
+#include "observable.h"
 #include "bfd.h"
 #include "symfile.h"
 #include "objfiles.h"
@@ -2591,12 +2592,45 @@ target_disconnect (const char *args, int from_tty)
   current_inferior ()->top_target ()->disconnect (args, from_tty);
 }
 
+/* RAII class to handle target_wait observers.
+   These observers allow the Insight GUI to monitor user interaction while
+   waiting on the target. */
+
+class scoped_target_wait
+{
+public:
+  explicit scoped_target_wait (ptid_t ptid)
+  {
+    m_event_ptid = null_ptid;
+    gdb::observers::target_pre_wait.notify (ptid);
+  }
+
+  ~scoped_target_wait ()
+  {
+    gdb::observers::target_post_wait.notify (m_event_ptid);
+  }
+
+  void set (ptid_t event_ptid) noexcept
+  {
+    m_event_ptid = event_ptid;
+  }
+
+  ptid_t get () const noexcept
+  {
+    return m_event_ptid;
+  }
+
+private:
+  ptid_t m_event_ptid;
+};
+
 /* See target/target.h.  */
 
 ptid_t
 target_wait (ptid_t ptid, struct target_waitstatus *status,
 	     target_wait_flags options)
 {
+  scoped_target_wait event_ptid(ptid);
   target_ops *target = current_inferior ()->top_target ();
   process_stratum_target *proc_target = current_inferior ()->process_target ();
 
@@ -2605,7 +2639,8 @@ target_wait (ptid_t ptid, struct target_waitstatus *status,
   if (!target->can_async_p ())
     gdb_assert ((options & TARGET_WNOHANG) == 0);
 
-  return target->wait (ptid, status, options);
+  event_ptid.set (target->wait (ptid, status, options));
+  return event_ptid.get ();
 }
 
 /* See target.h.  */
diff --git a/gdb/top.c b/gdb/top.c
index 0c49f4f9eb4..368166720ac 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -249,13 +249,6 @@ void (*deprecated_readline_end_hook) (void);
 void (*deprecated_attach_hook) (void);
 void (*deprecated_detach_hook) (void);
 
-/* Called when going to wait for the target.  Usually allows the GUI
-   to run while waiting for target events.  */
-
-ptid_t (*deprecated_target_wait_hook) (ptid_t ptid,
-				       struct target_waitstatus *status,
-				       int options);
-
 /* Used by UI as a wrapper around command execution.  May do various
    things like enabling/disabling buttons, etc...  */
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH] Replace deprecated_target_wait_hook by observers
@ 2022-03-12 12:41 patrick
  2022-03-14 13:53 ` Tom Tromey
  0 siblings, 1 reply; 9+ messages in thread
From: patrick @ 2022-03-12 12:41 UTC (permalink / raw)
  To: gdb-patches

From: Patrick Monnerat <patrick@monnerat.net>

Commit b60cea7 (Make target_wait options use enum flags) broke
deprecated_target_wait_hook usage: there's a commit comment telling
this hook has not been converted.

Rather than trying to mend it, this patch replaces the hook by two
target_wait observers:

target_pre_wait (ptid_t ptid)
target_post_wait (ptid_t event_ptid)

Upon target_wait entry, target_pre_wait is notified with the ptid
passed to target_wait. Upon exit, target_post_wait is notified with
the event ptid returned by target_wait. Should an exception occur,
event_ptid is null_ptid.

This change benefits to Insight (out-of-tree): there's no real use of the
late hook in gdb itself.
---
 gdb/infrun.c     | 15 +++------------
 gdb/infrun.h     |  5 ++---
 gdb/interps.c    |  1 -
 gdb/observable.c |  2 ++
 gdb/observable.h |  6 ++++++
 gdb/target.c     | 14 +++++++++++++-
 gdb/top.c        |  7 -------
 7 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index e3c1db73749..bc6521c8ec6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -367,7 +367,7 @@ show_stop_on_solib_events (struct ui_file *file, int from_tty,
 static bool stop_print_frame;
 
 /* This is a cached copy of the target/ptid/waitstatus of the last
-   event returned by target_wait()/deprecated_target_wait_hook().
+   event returned by target_wait().
    This information is returned by get_last_target_status().  */
 static process_stratum_target *target_last_proc_target;
 static ptid_t target_last_wait_ptid;
@@ -3515,7 +3515,6 @@ static ptid_t
 do_target_wait_1 (inferior *inf, ptid_t ptid,
 		  target_waitstatus *status, target_wait_flags options)
 {
-  ptid_t event_ptid;
   struct thread_info *tp;
 
   /* We know that we are looking for an event in the target of inferior
@@ -3630,12 +3629,7 @@ do_target_wait_1 (inferior *inf, ptid_t ptid,
   if (!target_can_async_p ())
     options &= ~TARGET_WNOHANG;
 
-  if (deprecated_target_wait_hook)
-    event_ptid = deprecated_target_wait_hook (ptid, status, options);
-  else
-    event_ptid = target_wait (ptid, status, options);
-
-  return event_ptid;
+  return target_wait (ptid, status, options);
 }
 
 /* Wrapper for target_wait that first checks whether threads have
@@ -4591,10 +4585,7 @@ poll_one_curr_target (struct target_waitstatus *ws)
      don't get any event.  */
   target_dcache_invalidate ();
 
-  if (deprecated_target_wait_hook)
-    event_ptid = deprecated_target_wait_hook (minus_one_ptid, ws, TARGET_WNOHANG);
-  else
-    event_ptid = target_wait (minus_one_ptid, ws, TARGET_WNOHANG);
+  event_ptid = target_wait (minus_one_ptid, ws, TARGET_WNOHANG);
 
   if (debug_infrun)
     print_target_wait_results (minus_one_ptid, event_ptid, *ws);
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 3e84805accb..1209b4188e9 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -124,9 +124,8 @@ extern process_stratum_target *user_visible_resume_target (ptid_t resume_ptid);
 extern int normal_stop (void);
 
 /* Return the cached copy of the last target/ptid/waitstatus returned
-   by target_wait()/deprecated_target_wait_hook().  The data is
-   actually cached by handle_inferior_event(), which gets called
-   immediately after target_wait()/deprecated_target_wait_hook().  */
+   by target_wait().  The data is actually cached by handle_inferior_event(),
+   which gets called immediately after target_wait().  */
 extern void get_last_target_status (process_stratum_target **target,
 				    ptid_t *ptid,
 				    struct target_waitstatus *status);
diff --git a/gdb/interps.c b/gdb/interps.c
index a475d8790c9..b8df3d781e7 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -357,7 +357,6 @@ clear_interpreter_hooks (void)
   deprecated_readline_hook = 0;
   deprecated_readline_end_hook = 0;
   deprecated_context_hook = 0;
-  deprecated_target_wait_hook = 0;
   deprecated_call_command_hook = 0;
   deprecated_error_begin_hook = 0;
 }
diff --git a/gdb/observable.c b/gdb/observable.c
index 78f315793b6..afe81394594 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -79,6 +79,8 @@ DEFINE_OBSERVABLE (styling_changed);
 DEFINE_OBSERVABLE (current_source_symtab_and_line_changed);
 DEFINE_OBSERVABLE (gdb_exiting);
 DEFINE_OBSERVABLE (connection_removed);
+DEFINE_OBSERVABLE (target_pre_wait);
+DEFINE_OBSERVABLE (target_post_wait);
 
 } /* namespace observers */
 } /* namespace gdb */
diff --git a/gdb/observable.h b/gdb/observable.h
index 0cdf4767f04..f426c1a761f 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -256,6 +256,12 @@ extern observable<int> gdb_exiting;
 /* When a connection is removed.  */
 extern observable<process_stratum_target */* target */> connection_removed;
 
+/* About to enter target_wait (). */
+extern observable <ptid_t /* ptid */> target_pre_wait;
+
+/* About to leave target_wait (). */
+extern observable <ptid_t /* event_ptid */> target_post_wait;
+
 } /* namespace observers */
 
 } /* namespace gdb */
diff --git a/gdb/target.c b/gdb/target.c
index 658698b4e2b..e3df9275c9a 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -26,6 +26,7 @@
 #include "symtab.h"
 #include "inferior.h"
 #include "infrun.h"
+#include "observable.h"
 #include "bfd.h"
 #include "symfile.h"
 #include "objfiles.h"
@@ -2609,7 +2610,18 @@ target_wait (ptid_t ptid, struct target_waitstatus *status,
   if (!target_can_async_p (target))
     gdb_assert ((options & TARGET_WNOHANG) == 0);
 
-  return target->wait (ptid, status, options);
+  try
+    {
+      gdb::observers::target_pre_wait.notify (ptid);
+      ptid_t event_ptid = target->wait (ptid, status, options);
+      gdb::observers::target_post_wait.notify (event_ptid);
+      return event_ptid;
+    }
+  catch (...)
+    {
+      gdb::observers::target_post_wait.notify (null_ptid);
+      throw;
+    }
 }
 
 /* See target.h.  */
diff --git a/gdb/top.c b/gdb/top.c
index a94ed5cebdb..7831b4f96ca 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -247,13 +247,6 @@ void (*deprecated_readline_end_hook) (void);
 void (*deprecated_attach_hook) (void);
 void (*deprecated_detach_hook) (void);
 
-/* Called when going to wait for the target.  Usually allows the GUI
-   to run while waiting for target events.  */
-
-ptid_t (*deprecated_target_wait_hook) (ptid_t ptid,
-				       struct target_waitstatus *status,
-				       int options);
-
 /* Used by UI as a wrapper around command execution.  May do various
    things like enabling/disabling buttons, etc...  */
 
-- 
2.35.1



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

end of thread, other threads:[~2022-03-14 14:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-26 14:40 [PATCH] Replace deprecated_target_wait_hook by observers Patrick Monnerat
2021-09-04 11:58 ` [PING] " Patrick Monnerat
2021-09-07 15:08 ` [PATCH] " Simon Marchi
2021-09-23 20:10   ` Tom Tromey
2021-10-25 19:02     ` Simon Marchi
2021-10-27 18:39       ` Tom Tromey
2022-03-12 12:41 patrick
2022-03-14 13:53 ` Tom Tromey
2022-03-14 14:16   ` Patrick Monnerat

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).