[RFA] Remove make_cleanup_unpush_target

Message ID 20180329152215.3783-1-tom@tromey.com
State New
Headers show
Series
  • [RFA] Remove make_cleanup_unpush_target
Related show

Commit Message

Tom Tromey March 29, 2018, 3:22 p.m.
This removes make_cleanup_unpush_target, replacing it with a
unique_ptr.  This may seem odd, because the object in question is not
actually freed, but unique_ptr provided the necessary functionality.

Tested by the buildbot.

gdb/ChangeLog
2018-03-29  Tom Tromey  <tom@tromey.com>

	* utils.h (make_cleanup_unpush_target): Remove.
	* inf-ptrace.c (struct target_unpusher): New.
	(target_unpush_up) New typedef.
	(inf_ptrace_create_inferior, inf_ptrace_attach): Use
	target_unpush_up.
	* utils.c (do_unpush_target, make_cleanup_unpush_target): Remove.
---
 gdb/ChangeLog    |  9 +++++++++
 gdb/inf-ptrace.c | 28 ++++++++++++++++++++++------
 gdb/utils.c      | 18 ------------------
 gdb/utils.h      |  3 ---
 4 files changed, 31 insertions(+), 27 deletions(-)

-- 
2.13.6

Comments

Simon Marchi March 30, 2018, 4:10 p.m. | #1
On 2018-03-29 11:22, Tom Tromey wrote:
> This removes make_cleanup_unpush_target, replacing it with a

> unique_ptr.  This may seem odd, because the object in question is not

> actually freed, but unique_ptr provided the necessary functionality.

> 

> Tested by the buildbot.


I was not sure about the usage of unique_ptr at first, but I guess it 
works just as well as writing our own object.  So it LGTM.

Simon

Patch

diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index a290cd81db..e20388658f 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -36,6 +36,22 @@ 
 
 
 
+/* A unique_ptr helper to unpush a target.  */
+
+struct target_unpusher
+{
+  void operator() (struct target_ops *ops) const
+  {
+    unpush_target (ops);
+  }
+};
+
+/* A unique_ptr that unpushes a target on destruction.  */
+
+typedef std::unique_ptr<struct target_ops, target_unpusher> target_unpush_up;
+
+
+
 #ifdef PT_GET_PROCESS_STATE
 
 /* Target hook for follow_fork.  On entry and at return inferior_ptid is
@@ -101,13 +117,13 @@  inf_ptrace_create_inferior (struct target_ops *ops,
   /* Do not change either targets above or the same target if already present.
      The reason is the target stack is shared across multiple inferiors.  */
   int ops_already_pushed = target_is_pushed (ops);
-  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
 
+  target_unpush_up unpusher;
   if (! ops_already_pushed)
     {
       /* Clear possible core file with its process_stratum.  */
       push_target (ops);
-      make_cleanup_unpush_target (ops);
+      unpusher.reset (ops);
     }
 
   pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL,
@@ -119,7 +135,7 @@  inf_ptrace_create_inferior (struct target_ops *ops,
      pid shouldn't change.  */
   add_thread_silent (ptid);
 
-  discard_cleanups (back_to);
+  unpusher.release ();
 
   gdb_startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
 
@@ -174,19 +190,19 @@  inf_ptrace_attach (struct target_ops *ops, const char *args, int from_tty)
   /* Do not change either targets above or the same target if already present.
      The reason is the target stack is shared across multiple inferiors.  */
   int ops_already_pushed = target_is_pushed (ops);
-  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
 
   pid = parse_pid_to_attach (args);
 
   if (pid == getpid ())		/* Trying to masturbate?  */
     error (_("I refuse to debug myself!"));
 
+  target_unpush_up unpusher;
   if (! ops_already_pushed)
     {
       /* target_pid_to_str already uses the target.  Also clear possible core
 	 file with its process_stratum.  */
       push_target (ops);
-      make_cleanup_unpush_target (ops);
+      unpusher.reset (ops);
     }
 
   if (from_tty)
@@ -221,7 +237,7 @@  inf_ptrace_attach (struct target_ops *ops, const char *args, int from_tty)
      target, it should decorate the ptid later with more info.  */
   add_thread_silent (inferior_ptid);
 
-  discard_cleanups (back_to);
+  unpusher.release ();
 }
 
 #ifdef PT_GET_PROCESS_STATE
diff --git a/gdb/utils.c b/gdb/utils.c
index ee31f39661..ee19fed172 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -141,24 +141,6 @@  show_pagination_enabled (struct ui_file *file, int from_tty,
    because while they use the "cleanup API" they are not part of the
    "cleanup API".  */
 
-/* Helper for make_cleanup_unpush_target.  */
-
-static void
-do_unpush_target (void *arg)
-{
-  struct target_ops *ops = (struct target_ops *) arg;
-
-  unpush_target (ops);
-}
-
-/* Return a new cleanup that unpushes OPS.  */
-
-struct cleanup *
-make_cleanup_unpush_target (struct target_ops *ops)
-{
-  return make_cleanup (do_unpush_target, ops);
-}
-
 /* Helper for make_cleanup_value_free_to_mark.  */
 
 static void
diff --git a/gdb/utils.h b/gdb/utils.h
index 6ff18568fe..0de0fe2baa 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -244,9 +244,6 @@  private:
 
 /* For make_cleanup_close see common/filestuff.h.  */
 
-struct target_ops;
-extern struct cleanup *make_cleanup_unpush_target (struct target_ops *ops);
-
 extern struct cleanup *make_cleanup_value_free_to_mark (struct value *);
 
 /* A deleter for a hash table.  */