public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Remove target_close
@ 2023-07-10 17:01 Tom Tromey
  2023-07-10 20:17 ` Andrew Burgess
  0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2023-07-10 17:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

I noticed that target_close is only called in two places:
solib-svr4.c, and target_ops_ref_policy::decref.

This patch fixes the former by changing target_bfd_reopen to return a
target_ops_up and then fixing the sole caller.  Then it removes
target_close by inlining its body into the decref method.

The advantage of this approach is that targets are now automatically
managed.

Regression tested on x86-64 Fedora 38.
---
 gdb/bfd-target.c |  4 ++--
 gdb/bfd-target.h |  2 +-
 gdb/solib-svr4.c | 16 ++++++----------
 gdb/target.c     | 25 ++++++++++---------------
 gdb/target.h     |  9 ---------
 5 files changed, 19 insertions(+), 37 deletions(-)

diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c
index 6b8a839b286..9e1b980957c 100644
--- a/gdb/bfd-target.c
+++ b/gdb/bfd-target.c
@@ -94,10 +94,10 @@ target_bfd::target_bfd (const gdb_bfd_ref_ptr &abfd)
 {
 }
 
-target_ops *
+target_ops_up
 target_bfd_reopen (const gdb_bfd_ref_ptr &abfd)
 {
-  return new target_bfd (abfd);
+  return target_ops_up (new target_bfd (abfd));
 }
 
 void
diff --git a/gdb/bfd-target.h b/gdb/bfd-target.h
index d77f0367fd5..eb33374c6ea 100644
--- a/gdb/bfd-target.h
+++ b/gdb/bfd-target.h
@@ -25,6 +25,6 @@
 struct target_ops;
 
 /* Given an existing BFD, re-open it as a "struct target_ops".  */
-struct target_ops *target_bfd_reopen (const gdb_bfd_ref_ptr &bfd);
+target_ops_up target_bfd_reopen (const gdb_bfd_ref_ptr &bfd);
 
 #endif
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 0ef83726d62..f1fa437e31a 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2425,7 +2425,7 @@ enable_break (struct svr4_info *info, int from_tty)
       CORE_ADDR load_addr = 0;
       int load_addr_found = 0;
       int loader_found_in_list = 0;
-      struct target_ops *tmp_bfd_target;
+      target_ops_up tmp_bfd_target;
 
       sym_addr = 0;
 
@@ -2482,8 +2482,8 @@ enable_break (struct svr4_info *info, int from_tty)
 	    if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
 	      {
 		CORE_ADDR space_size = (CORE_ADDR) 1 << addr_bit;
-		CORE_ADDR tmp_entry_point = exec_entry_point (tmp_bfd.get (),
-							      tmp_bfd_target);
+		CORE_ADDR tmp_entry_point
+		  = exec_entry_point (tmp_bfd.get (), tmp_bfd_target.get ());
 
 		gdb_assert (load_addr < space_size);
 
@@ -2512,7 +2512,8 @@ enable_break (struct svr4_info *info, int from_tty)
 					inferior_ptid, target_gdbarch ());
 
 	  load_addr = (regcache_read_pc (regcache)
-		       - exec_entry_point (tmp_bfd.get (), tmp_bfd_target));
+		       - exec_entry_point (tmp_bfd.get (),
+					   tmp_bfd_target.get ()));
 	}
 
       if (!loader_found_in_list)
@@ -2564,12 +2565,7 @@ enable_break (struct svr4_info *info, int from_tty)
 	   target, this will always produce an unrelocated value.  */
 	sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch (),
 						       sym_addr,
-						       tmp_bfd_target);
-
-      /* We're done with both the temporary bfd and target.  Closing
-	 the target closes the underlying bfd, because it holds the
-	 only remaining reference.  */
-      target_close (tmp_bfd_target);
+						       tmp_bfd_target.get ());
 
       if (sym_addr != 0)
 	{
diff --git a/gdb/target.c b/gdb/target.c
index fecbc89d3ca..16f43d072cd 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1173,7 +1173,16 @@ target_ops_ref_policy::decref (target_ops *t)
     {
       if (t->stratum () == process_stratum)
 	connection_list_remove (as_process_stratum_target (t));
-      target_close (t);
+
+      for (inferior *inf : all_inferiors ())
+	gdb_assert (!inf->target_is_pushed (t));
+
+      fileio_handles_invalidate_target (t);
+
+      t->close ();
+
+      if (targetdebug)
+	gdb_printf (gdb_stdlog, "closing target\n");
     }
 }
 
@@ -3752,20 +3761,6 @@ debug_target::info () const
 
 \f
 
-void
-target_close (struct target_ops *targ)
-{
-  for (inferior *inf : all_inferiors ())
-    gdb_assert (!inf->target_is_pushed (targ));
-
-  fileio_handles_invalidate_target (targ);
-
-  targ->close ();
-
-  if (targetdebug)
-    gdb_printf (gdb_stdlog, "target_close ()\n");
-}
-
 int
 target_thread_alive (ptid_t ptid)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 5f9a2a2d97a..6ae400e2cc2 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1410,15 +1410,6 @@ extern target_ops *get_dummy_target ();
 
 extern const char *target_shortname ();
 
-/* Does whatever cleanup is required for a target that we are no
-   longer going to be calling.  This routine is automatically always
-   called after popping the target off the target stack - the target's
-   own methods are no longer available through the target vector.
-   Closing file descriptors and freeing all memory allocated memory are
-   typical things it should do.  */
-
-void target_close (struct target_ops *targ);
-
 /* Find the correct target to use for "attach".  If a target on the
    current stack supports attaching, then it is returned.  Otherwise,
    the default run target is returned.  */
-- 
2.41.0


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-10 21:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-10 17:01 [PATCH] Remove target_close Tom Tromey
2023-07-10 20:17 ` Andrew Burgess
2023-07-10 21:40   ` Tom Tromey

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