[RFA 0/5] remove some infrun-related cleanups

[RFA 0/5] remove some infrun-related cleanups

[Tom Tromey]

This series removes some cleanups from infrun.

Tested by the buildbot.

Tom

[RFA 1/5] Use new and delete for struct infcall_suspend_state

[Tom Tromey]

This changes infrun.c to use new and delete for infcall_suspend_state.
This enables the coming cleanups.

gdb/ChangeLog
2018-07-11  Tom Tromey  <tom@tromey.com>

	* infrun.c (struct infcall_suspend_state): Add initializers.
	(save_infcall_suspend_state): Use new.
	(discard_infcall_suspend_state): Use delete.
---
 gdb/ChangeLog |  6 ++++++
 gdb/infrun.c  | 12 ++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7c74c87d8c5..8c0d7b5f6a9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-07-11  Tom Tromey  <tom@tromey.com>
+
+	* infrun.c (struct infcall_suspend_state): Add initializers.
+	(save_infcall_suspend_state): Use new.
+	(discard_infcall_suspend_state): Use delete.
+
 2018-07-11  Alan Hayward  <alan.hayward@arm.com>
 
 	* target-descriptions.c (tdesc_register_bitsize): Rename.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index dd7e69e718e..da71f3132a0 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8810,18 +8810,18 @@ siginfo_make_value (struct gdbarch *gdbarch, struct internalvar *var,
 
 struct infcall_suspend_state
 {
-  struct thread_suspend_state thread_suspend;
+  struct thread_suspend_state thread_suspend {};
 
   /* Other fields:  */
-  readonly_detached_regcache *registers;
+  readonly_detached_regcache *registers = nullptr;
 
   /* Format of SIGINFO_DATA or NULL if it is not present.  */
-  struct gdbarch *siginfo_gdbarch;
+  struct gdbarch *siginfo_gdbarch = nullptr;
 
   /* The inferior format depends on SIGINFO_GDBARCH and it has a length of
      TYPE_LENGTH (gdbarch_get_siginfo_type ()).  For different gdbarch the
      content would be invalid.  */
-  gdb_byte *siginfo_data;
+  gdb_byte *siginfo_data = nullptr;
 };
 
 struct infcall_suspend_state *
@@ -8853,7 +8853,7 @@ save_infcall_suspend_state (void)
 	}
     }
 
-  inf_state = XCNEW (struct infcall_suspend_state);
+  inf_state = new struct infcall_suspend_state;
 
   if (siginfo_data)
     {
@@ -8919,7 +8919,7 @@ discard_infcall_suspend_state (struct infcall_suspend_state *inf_state)
 {
   delete inf_state->registers;
   xfree (inf_state->siginfo_data);
-  xfree (inf_state);
+  delete inf_state;
 }
 
 readonly_detached_regcache *
-- 
2.17.1

Re: [RFA 1/5] Use new and delete for struct infcall_suspend_state

[Simon Marchi]

On 2018-07-11 10:15, Tom Tromey wrote:
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index dd7e69e718e..da71f3132a0 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8810,18 +8810,18 @@ siginfo_make_value (struct gdbarch *gdbarch,
> struct internalvar *var,
> 
>  struct infcall_suspend_state
>  {
> -  struct thread_suspend_state thread_suspend;
> +  struct thread_suspend_state thread_suspend {};

Small nit: You could push the {} initialization to the 
thread_suspend_state class.  That is, initialize thread_suspend_state's 
fields directly.  You can then remove these {}, as well as on 
thread_info::suspend.

Simon

[RFA 4/5] Remove two infrun cleanups

[Tom Tromey]

This removes a couple of cleanups from infrun by introducing a couple
of unique_ptr specializations.

gdb/ChangeLog
2018-07-11  Tom Tromey  <tom@tromey.com>

	* inferior.h (struct infcall_suspend_state_deleter): New.
	(infcall_suspend_state_up): New typedef.
	(struct infcall_control_state_deleter): New.
	(infcall_control_state_up): New typedef.
	(make_cleanup_restore_infcall_suspend_state)
	(make_cleanup_restore_infcall_control_state): Don't declare.
	* infcall.c (call_function_by_hand_dummy): Update.
	* infrun.c (do_restore_infcall_suspend_state_cleanup)
	(make_cleanup_restore_infcall_suspend_state): Remove.
	(do_restore_infcall_control_state_cleanup)
	(make_cleanup_restore_infcall_control_state): Remove.
---
 gdb/ChangeLog  | 14 ++++++++++++++
 gdb/infcall.c  | 39 ++++++++++++++-------------------------
 gdb/inferior.h | 31 +++++++++++++++++++++++++++----
 gdb/infrun.c   | 26 --------------------------
 4 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 95d4f51f62c..12e27000151 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2018-07-11  Tom Tromey  <tom@tromey.com>
+
+	* inferior.h (struct infcall_suspend_state_deleter): New.
+	(infcall_suspend_state_up): New typedef.
+	(struct infcall_control_state_deleter): New.
+	(infcall_control_state_up): New typedef.
+	(make_cleanup_restore_infcall_suspend_state)
+	(make_cleanup_restore_infcall_control_state): Don't declare.
+	* infcall.c (call_function_by_hand_dummy): Update.
+	* infrun.c (do_restore_infcall_suspend_state_cleanup)
+	(make_cleanup_restore_infcall_suspend_state): Remove.
+	(do_restore_infcall_control_state_cleanup)
+	(make_cleanup_restore_infcall_control_state): Remove.
+
 2018-07-11  Tom Tromey  <tom@tromey.com>
 
 	* infrun.c (struct infcall_control_state): Add initializers.
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 172c26ac820..b7071a722f1 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -724,9 +724,6 @@ call_function_by_hand_dummy (struct value *function,
   struct type *target_values_type;
   unsigned char struct_return = 0, hidden_first_param_p = 0;
   CORE_ADDR struct_addr = 0;
-  struct infcall_control_state *inf_status;
-  struct cleanup *inf_status_cleanup;
-  struct infcall_suspend_state *caller_state;
   CORE_ADDR real_pc;
   CORE_ADDR bp_addr;
   struct frame_id dummy_id;
@@ -760,19 +757,16 @@ call_function_by_hand_dummy (struct value *function,
   if (!gdbarch_push_dummy_call_p (gdbarch))
     error (_("This target does not support function calls."));
 
-  /* A cleanup for the inferior status.
+  /* A holder for the inferior status.
      This is only needed while we're preparing the inferior function call.  */
-  inf_status = save_infcall_control_state ();
-  inf_status_cleanup
-    = make_cleanup_restore_infcall_control_state (inf_status);
+  infcall_control_state_up inf_status (save_infcall_control_state ());
 
   /* Save the caller's registers and other state associated with the
      inferior itself so that they can be restored once the
      callee returns.  To allow nested calls the registers are (further
-     down) pushed onto a dummy frame stack.  Include a cleanup (which
-     is tossed once the regcache has been pushed).  */
-  caller_state = save_infcall_suspend_state ();
-  make_cleanup_restore_infcall_suspend_state (caller_state);
+     down) pushed onto a dummy frame stack.  This unique pointer
+     is released once the regcache has been pushed).  */
+  infcall_suspend_state_up caller_state (save_infcall_suspend_state ());
 
   /* Ensure that the initial SP is correctly aligned.  */
   {
@@ -1133,15 +1127,10 @@ call_function_by_hand_dummy (struct value *function,
   if (unwind_on_terminating_exception_p)
     set_std_terminate_breakpoint ();
 
-  /* Discard both inf_status and caller_state cleanups.
-     From this point on we explicitly restore the associated state
-     or discard it.  */
-  discard_cleanups (inf_status_cleanup);
-
   /* Everything's ready, push all the info needed to restore the
      caller (and identify the dummy-frame) onto the dummy-frame
      stack.  */
-  dummy_frame_push (caller_state, &dummy_id, call_thread.get ());
+  dummy_frame_push (caller_state.release (), &dummy_id, call_thread.get ());
   if (dummy_dtor != NULL)
     register_dummy_frame_dtor (dummy_id, call_thread.get (),
 			       dummy_dtor, dummy_dtor_data);
@@ -1196,7 +1185,7 @@ call_function_by_hand_dummy (struct value *function,
 	       suspend state, and restore the inferior control
 	       state.  */
 	    dummy_frame_pop (dummy_id, call_thread.get ());
-	    restore_infcall_control_state (inf_status);
+	    restore_infcall_control_state (inf_status.release ());
 
 	    /* Get the return value.  */
 	    retval = sm->return_value;
@@ -1227,7 +1216,7 @@ call_function_by_hand_dummy (struct value *function,
       const char *name = get_function_name (funaddr,
                                             name_buf, sizeof (name_buf));
 
-      discard_infcall_control_state (inf_status);
+      discard_infcall_control_state (inf_status.release ());
 
       /* We could discard the dummy frame here if the program exited,
          but it will get garbage collected the next time the program is
@@ -1258,7 +1247,7 @@ When the function is done executing, GDB will silently stop."),
 
       /* If we try to restore the inferior status,
 	 we'll crash as the inferior is no longer running.  */
-      discard_infcall_control_state (inf_status);
+      discard_infcall_control_state (inf_status.release ());
 
       /* We could discard the dummy frame here given that the program exited,
          but it will get garbage collected the next time the program is
@@ -1280,7 +1269,7 @@ When the function is done executing, GDB will silently stop."),
 	 signal or breakpoint while our thread was running.
 	 There's no point in restoring the inferior status,
 	 we're in a different thread.  */
-      discard_infcall_control_state (inf_status);
+      discard_infcall_control_state (inf_status.release ());
       /* Keep the dummy frame record, if the user switches back to the
 	 thread with the hand-call, we'll need it.  */
       if (stopped_by_random_signal)
@@ -1321,7 +1310,7 @@ When the function is done executing, GDB will silently stop."),
 
 	      /* We also need to restore inferior status to that before the
 		 dummy call.  */
-	      restore_infcall_control_state (inf_status);
+	      restore_infcall_control_state (inf_status.release ());
 
 	      /* FIXME: Insert a bunch of wrap_here; name can be very
 		 long if it's a C++ name with arguments and stuff.  */
@@ -1339,7 +1328,7 @@ Evaluation of the expression containing the function\n\
 		 (default).
 		 Discard inferior status, we're not at the same point
 		 we started at.  */
-	      discard_infcall_control_state (inf_status);
+	      discard_infcall_control_state (inf_status.release ());
 
 	      /* FIXME: Insert a bunch of wrap_here; name can be very
 		 long if it's a C++ name with arguments and stuff.  */
@@ -1362,7 +1351,7 @@ When the function is done executing, GDB will silently stop."),
 
 	  /* We also need to restore inferior status to that before
 	     the dummy call.  */
-	  restore_infcall_control_state (inf_status);
+	  restore_infcall_control_state (inf_status.release ());
 
 	  error (_("\
 The program being debugged entered a std::terminate call, most likely\n\
@@ -1381,7 +1370,7 @@ will be abandoned."),
 	     Keep the dummy frame, the user may want to examine its state.
 	     Discard inferior status, we're not at the same point
 	     we started at.  */
-	  discard_infcall_control_state (inf_status);
+	  discard_infcall_control_state (inf_status.release ());
 
 	  /* The following error message used to say "The expression
 	     which contained the function call has been discarded."
diff --git a/gdb/inferior.h b/gdb/inferior.h
index c5dc324fa39..5591382079f 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -63,10 +63,33 @@ extern struct infcall_control_state *save_infcall_control_state (void);
 extern void restore_infcall_suspend_state (struct infcall_suspend_state *);
 extern void restore_infcall_control_state (struct infcall_control_state *);
 
-extern struct cleanup *make_cleanup_restore_infcall_suspend_state
-					    (struct infcall_suspend_state *);
-extern struct cleanup *make_cleanup_restore_infcall_control_state
-					    (struct infcall_control_state *);
+/* A deleter for infcall_suspend_state that calls
+   restore_infcall_suspend_state.  */
+struct infcall_suspend_state_deleter
+{
+  void operator() (struct infcall_suspend_state *state) const
+  {
+    restore_infcall_suspend_state (state);
+  }
+};
+
+/* A unique_ptr specialization for infcall_suspend_state.  */
+typedef std::unique_ptr<infcall_suspend_state, infcall_suspend_state_deleter>
+    infcall_suspend_state_up;
+
+/* A deleter for infcall_control_state that calls
+   restore_infcall_control_state.  */
+struct infcall_control_state_deleter
+{
+  void operator() (struct infcall_control_state *state) const
+  {
+    restore_infcall_control_state (state);
+  }
+};
+
+/* A unique_ptr specialization for infcall_control_state.  */
+typedef std::unique_ptr<infcall_control_state, infcall_control_state_deleter>
+    infcall_control_state_up;
 
 extern void discard_infcall_suspend_state (struct infcall_suspend_state *);
 extern void discard_infcall_control_state (struct infcall_control_state *);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3bc8dc03830..98b79e8c5ae 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8896,19 +8896,6 @@ restore_infcall_suspend_state (struct infcall_suspend_state *inf_state)
   discard_infcall_suspend_state (inf_state);
 }
 
-static void
-do_restore_infcall_suspend_state_cleanup (void *state)
-{
-  restore_infcall_suspend_state ((struct infcall_suspend_state *) state);
-}
-
-struct cleanup *
-make_cleanup_restore_infcall_suspend_state
-  (struct infcall_suspend_state *inf_state)
-{
-  return make_cleanup (do_restore_infcall_suspend_state_cleanup, inf_state);
-}
-
 void
 discard_infcall_suspend_state (struct infcall_suspend_state *inf_state)
 {
@@ -9033,19 +9020,6 @@ restore_infcall_control_state (struct infcall_control_state *inf_status)
   delete inf_status;
 }
 
-static void
-do_restore_infcall_control_state_cleanup (void *sts)
-{
-  restore_infcall_control_state ((struct infcall_control_state *) sts);
-}
-
-struct cleanup *
-make_cleanup_restore_infcall_control_state
-  (struct infcall_control_state *inf_status)
-{
-  return make_cleanup (do_restore_infcall_control_state_cleanup, inf_status);
-}
-
 void
 discard_infcall_control_state (struct infcall_control_state *inf_status)
 {
-- 
2.17.1

[RFA 2/5] Remove cleanup from infrun.c

[Tom Tromey]

This removes a cleanup from infrun.c by taking advantage of the
previous patch to introduce a use of unique_xmalloc_ptr.

gdb/ChangeLog
2018-07-11  Tom Tromey  <tom@tromey.com>

	* infrun.c (struct infcall_suspend_state) <registers>: Now a
	unique_ptr.
	<siginfo_data>: Now a unique_xmalloc_ptr.
	(save_infcall_suspend_state, restore_infcall_suspend_state)
	(discard_infcall_suspend_state)
	(get_infcall_suspend_state_regcache): Update.
---
 gdb/ChangeLog |  9 +++++++++
 gdb/infrun.c  | 29 +++++++++++------------------
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8c0d7b5f6a9..d46ccf53999 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2018-07-11  Tom Tromey  <tom@tromey.com>
+
+	* infrun.c (struct infcall_suspend_state) <registers>: Now a
+	unique_ptr.
+	<siginfo_data>: Now a unique_xmalloc_ptr.
+	(save_infcall_suspend_state, restore_infcall_suspend_state)
+	(discard_infcall_suspend_state)
+	(get_infcall_suspend_state_regcache): Update.
+
 2018-07-11  Tom Tromey  <tom@tromey.com>
 
 	* infrun.c (struct infcall_suspend_state): Add initializers.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index da71f3132a0..1157b266b1a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8813,7 +8813,7 @@ struct infcall_suspend_state
   struct thread_suspend_state thread_suspend {};
 
   /* Other fields:  */
-  readonly_detached_regcache *registers = nullptr;
+  std::unique_ptr<readonly_detached_regcache> registers;
 
   /* Format of SIGINFO_DATA or NULL if it is not present.  */
   struct gdbarch *siginfo_gdbarch = nullptr;
@@ -8821,7 +8821,7 @@ struct infcall_suspend_state
   /* The inferior format depends on SIGINFO_GDBARCH and it has a length of
      TYPE_LENGTH (gdbarch_get_siginfo_type ()).  For different gdbarch the
      content would be invalid.  */
-  gdb_byte *siginfo_data = nullptr;
+  gdb::unique_xmalloc_ptr<gdb_byte> siginfo_data;
 };
 
 struct infcall_suspend_state *
@@ -8831,25 +8831,20 @@ save_infcall_suspend_state (void)
   struct thread_info *tp = inferior_thread ();
   struct regcache *regcache = get_current_regcache ();
   struct gdbarch *gdbarch = regcache->arch ();
-  gdb_byte *siginfo_data = NULL;
+  gdb::unique_xmalloc_ptr<gdb_byte> siginfo_data;
 
   if (gdbarch_get_siginfo_type_p (gdbarch))
     {
       struct type *type = gdbarch_get_siginfo_type (gdbarch);
       size_t len = TYPE_LENGTH (type);
-      struct cleanup *back_to;
 
-      siginfo_data = (gdb_byte *) xmalloc (len);
-      back_to = make_cleanup (xfree, siginfo_data);
+      siginfo_data.reset ((gdb_byte *) xmalloc (len));
 
       if (target_read (current_top_target (), TARGET_OBJECT_SIGNAL_INFO, NULL,
-		       siginfo_data, 0, len) == len)
-	discard_cleanups (back_to);
-      else
+		       siginfo_data.get (), 0, len) != len)
 	{
 	  /* Errors ignored.  */
-	  do_cleanups (back_to);
-	  siginfo_data = NULL;
+	  siginfo_data.reset (nullptr);
 	}
     }
 
@@ -8858,7 +8853,7 @@ save_infcall_suspend_state (void)
   if (siginfo_data)
     {
       inf_state->siginfo_gdbarch = gdbarch;
-      inf_state->siginfo_data = siginfo_data;
+      inf_state->siginfo_data = std::move (siginfo_data);
     }
 
   inf_state->thread_suspend = tp->suspend;
@@ -8867,7 +8862,7 @@ save_infcall_suspend_state (void)
      GDB_SIGNAL_0 anyway.  */
   tp->suspend.stop_signal = GDB_SIGNAL_0;
 
-  inf_state->registers = new readonly_detached_regcache (*regcache);
+  inf_state->registers.reset (new readonly_detached_regcache (*regcache));
 
   return inf_state;
 }
@@ -8889,14 +8884,14 @@ restore_infcall_suspend_state (struct infcall_suspend_state *inf_state)
 
       /* Errors ignored.  */
       target_write (current_top_target (), TARGET_OBJECT_SIGNAL_INFO, NULL,
-		    inf_state->siginfo_data, 0, TYPE_LENGTH (type));
+		    inf_state->siginfo_data.get (), 0, TYPE_LENGTH (type));
     }
 
   /* The inferior can be gone if the user types "print exit(0)"
      (and perhaps other times).  */
   if (target_has_execution)
     /* NB: The register write goes through to the target.  */
-    regcache->restore (inf_state->registers);
+    regcache->restore (inf_state->registers.get ());
 
   discard_infcall_suspend_state (inf_state);
 }
@@ -8917,15 +8912,13 @@ make_cleanup_restore_infcall_suspend_state
 void
 discard_infcall_suspend_state (struct infcall_suspend_state *inf_state)
 {
-  delete inf_state->registers;
-  xfree (inf_state->siginfo_data);
   delete inf_state;
 }
 
 readonly_detached_regcache *
 get_infcall_suspend_state_regcache (struct infcall_suspend_state *inf_state)
 {
-  return inf_state->registers;
+  return inf_state->registers.get ();
 }
 
 /* infcall_control_state contains state regarding gdb's control of the
-- 
2.17.1

[RFA 5/5] Remove release_stop_context_cleanup

[Tom Tromey]

This removes release_stop_context_cleanup, replacing it with a
stop_context destructor.  It also mildly c++-ifies this struct.

gdb/ChangeLog
2018-07-11  Tom Tromey  <tom@tromey.com>

	* infrun.c (struct stop_context): Declare constructor,
	destructor, "changed" method.
	(stop_context::stop_context): Rename from save_stop_context.
	(stop_context::~stop_context): Rename from
	release_stop_context_cleanup.
	(normal_stop): Update.
	(stop_context::changed): Rename from stop_context_changed.  Return
	bool.
---
 gdb/ChangeLog | 11 ++++++++
 gdb/infrun.c  | 74 +++++++++++++++++++++++----------------------------
 2 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 12e27000151..1bc7853b8e5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2018-07-11  Tom Tromey  <tom@tromey.com>
+
+	* infrun.c (struct stop_context): Declare constructor,
+	destructor, "changed" method.
+	(stop_context::stop_context): Rename from save_stop_context.
+	(stop_context::~stop_context): Rename from
+	release_stop_context_cleanup.
+	(normal_stop): Update.
+	(stop_context::changed): Rename from stop_context_changed.  Return
+	bool.
+
 2018-07-11  Tom Tromey  <tom@tromey.com>
 
 	* inferior.h (struct infcall_suspend_state_deleter): New.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 98b79e8c5ae..bc69123c42e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8098,6 +8098,13 @@ maybe_remove_breakpoints (void)
 
 struct stop_context
 {
+  stop_context ();
+  ~stop_context ();
+
+  DISABLE_COPY_AND_ASSIGN (stop_context);
+
+  bool changed () const;
+
   /* The stop ID.  */
   ULONGEST stop_id;
 
@@ -8113,59 +8120,50 @@ struct stop_context
   int inf_num;
 };
 
-/* Returns a new stop context.  If stopped for a thread event, this
+/* Initializes a new stop context.  If stopped for a thread event, this
    takes a strong reference to the thread.  */
 
-static struct stop_context *
-save_stop_context (void)
+stop_context::stop_context ()
 {
-  struct stop_context *sc = XNEW (struct stop_context);
-
-  sc->stop_id = get_stop_id ();
-  sc->ptid = inferior_ptid;
-  sc->inf_num = current_inferior ()->num;
+  stop_id = get_stop_id ();
+  ptid = inferior_ptid;
+  inf_num = current_inferior ()->num;
 
   if (inferior_ptid != null_ptid)
     {
       /* Take a strong reference so that the thread can't be deleted
 	 yet.  */
-      sc->thread = inferior_thread ();
-      sc->thread->incref ();
+      thread = inferior_thread ();
+      thread->incref ();
     }
   else
-    sc->thread = NULL;
-
-  return sc;
+    thread = NULL;
 }
 
 /* Release a stop context previously created with save_stop_context.
    Releases the strong reference to the thread as well. */
 
-static void
-release_stop_context_cleanup (void *arg)
+stop_context::~stop_context ()
 {
-  struct stop_context *sc = (struct stop_context *) arg;
-
-  if (sc->thread != NULL)
-    sc->thread->decref ();
-  xfree (sc);
+  if (thread != NULL)
+    thread->decref ();
 }
 
 /* Return true if the current context no longer matches the saved stop
    context.  */
 
-static int
-stop_context_changed (struct stop_context *prev)
-{
-  if (prev->ptid != inferior_ptid)
-    return 1;
-  if (prev->inf_num != current_inferior ()->num)
-    return 1;
-  if (prev->thread != NULL && prev->thread->state != THREAD_STOPPED)
-    return 1;
-  if (get_stop_id () != prev->stop_id)
-    return 1;
-  return 0;
+bool
+stop_context::changed () const
+{
+  if (ptid != inferior_ptid)
+    return true;
+  if (inf_num != current_inferior ()->num)
+    return true;
+  if (thread != NULL && thread->state != THREAD_STOPPED)
+    return true;
+  if (get_stop_id () != stop_id)
+    return true;
+  return false;
 }
 
 /* See infrun.h.  */
@@ -8308,9 +8306,7 @@ normal_stop (void)
      of stop_command's pre-hook not existing).  */
   if (stop_command != NULL)
     {
-      struct stop_context *saved_context = save_stop_context ();
-      struct cleanup *old_chain
-	= make_cleanup (release_stop_context_cleanup, saved_context);
+      stop_context saved_context;
 
       TRY
 	{
@@ -8328,12 +8324,8 @@ normal_stop (void)
 	 gone.  Likewise if the command switches thread or inferior --
 	 the observers would print a stop for the wrong
 	 thread/inferior.  */
-      if (stop_context_changed (saved_context))
-	{
-	  do_cleanups (old_chain);
-	  return 1;
-	}
-      do_cleanups (old_chain);
+      if (saved_context.changed ())
+	return 1;
     }
 
   /* Notify observers about the stop.  This is where the interpreters
-- 
2.17.1

[RFA 3/5] Use new and delete for struct infcall_control_state

[Tom Tromey]

This changes infrun.c to use new and delete for infcall_control_state.

gdb/ChangeLog
2018-07-11  Tom Tromey  <tom@tromey.com>

	* infrun.c (struct infcall_control_state): Add initializers.
	(save_infcall_control_state): Use new.
	(restore_infcall_control_state, discard_infcall_control_state):
	Use delete.
---
 gdb/ChangeLog |  7 +++++++
 gdb/infrun.c  | 17 ++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d46ccf53999..95d4f51f62c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-07-11  Tom Tromey  <tom@tromey.com>
+
+	* infrun.c (struct infcall_control_state): Add initializers.
+	(save_infcall_control_state): Use new.
+	(restore_infcall_control_state, discard_infcall_control_state):
+	Use delete.
+
 2018-07-11  Tom Tromey  <tom@tromey.com>
 
 	* infrun.c (struct infcall_suspend_state) <registers>: Now a
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1157b266b1a..3bc8dc03830 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8927,15 +8927,15 @@ get_infcall_suspend_state_regcache (struct infcall_suspend_state *inf_state)
 
 struct infcall_control_state
 {
-  struct thread_control_state thread_control;
-  struct inferior_control_state inferior_control;
+  struct thread_control_state thread_control {};
+  struct inferior_control_state inferior_control {};
 
   /* Other fields:  */
-  enum stop_stack_kind stop_stack_dummy;
-  int stopped_by_random_signal;
+  enum stop_stack_kind stop_stack_dummy = STOP_NONE;
+  int stopped_by_random_signal = 0;
 
   /* ID if the selected frame when the inferior function call was made.  */
-  struct frame_id selected_frame_id;
+  struct frame_id selected_frame_id {};
 };
 
 /* Save all of the information associated with the inferior<==>gdb
@@ -8944,8 +8944,7 @@ struct infcall_control_state
 struct infcall_control_state *
 save_infcall_control_state (void)
 {
-  struct infcall_control_state *inf_status =
-    XNEW (struct infcall_control_state);
+  struct infcall_control_state *inf_status = new struct infcall_control_state;
   struct thread_info *tp = inferior_thread ();
   struct inferior *inf = current_inferior ();
 
@@ -9031,7 +9030,7 @@ restore_infcall_control_state (struct infcall_control_state *inf_status)
       END_CATCH
     }
 
-  xfree (inf_status);
+  delete inf_status;
 }
 
 static void
@@ -9061,7 +9060,7 @@ discard_infcall_control_state (struct infcall_control_state *inf_status)
   /* See save_infcall_control_state for info on stop_bpstat.  */
   bpstat_clear (&inf_status->thread_control.stop_bpstat);
 
-  xfree (inf_status);
+  delete inf_status;
 }
 \f
 /* See infrun.h.  */
-- 
2.17.1

Re: [RFA 3/5] Use new and delete for struct infcall_control_state

[Simon Marchi]

On 2018-07-11 10:15, Tom Tromey wrote:
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 1157b266b1a..3bc8dc03830 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8927,15 +8927,15 @@ get_infcall_suspend_state_regcache (struct
> infcall_suspend_state *inf_state)
> 
>  struct infcall_control_state
>  {
> -  struct thread_control_state thread_control;
> -  struct inferior_control_state inferior_control;
> +  struct thread_control_state thread_control {};
> +  struct inferior_control_state inferior_control {};

Same here, you could remove the {} here and make the 
thread/inferior_control_state objects initialize themselves.  
thread_info::control and thread_info::suspend then won't need to be 
explicitly initialized.

Simon

Re: [RFA 3/5] Use new and delete for struct infcall_control_state

[Simon Marchi]

On 2018-07-11 11:36, Simon Marchi wrote:
> On 2018-07-11 10:15, Tom Tromey wrote:
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 1157b266b1a..3bc8dc03830 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -8927,15 +8927,15 @@ get_infcall_suspend_state_regcache (struct
>> infcall_suspend_state *inf_state)
>> 
>>  struct infcall_control_state
>>  {
>> -  struct thread_control_state thread_control;
>> -  struct inferior_control_state inferior_control;
>> +  struct thread_control_state thread_control {};
>> +  struct inferior_control_state inferior_control {};
> 
> Same here, you could remove the {} here and make the
> thread/inferior_control_state objects initialize themselves.
> thread_info::control and thread_info::suspend then won't need to be
> explicitly initialized.

You probably got it anyway, but the last sentence should have said 
inferior::control and thread_info::control.

Simon

Re: [RFA 0/5] remove some infrun-related cleanups

[Pedro Alves]

On 07/11/2018 03:15 PM, Tom Tromey wrote:
> This series removes some cleanups from infrun.
> 
> Tested by the buildbot.

OK.

Thanks,
Pedro Alves

Re: [RFA 0/5] remove some infrun-related cleanups

[Simon Marchi]

On 2018-07-11 10:54, Pedro Alves wrote:
> On 07/11/2018 03:15 PM, Tom Tromey wrote:
>> This series removes some cleanups from infrun.
>> 
>> Tested by the buildbot.
> 
> OK.
> 
> Thanks,
> Pedro Alves

Small obvious suggestion of making save_infcall_suspend_state and 
save_infcall_control_state return unique pointers.

Simon

Re: [RFA 0/5] remove some infrun-related cleanups

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Small obvious suggestion of making save_infcall_suspend_state and
Simon> save_infcall_control_state return unique pointers.

I've tacked on a new patch to the end of the series to do this.
I'll send it all again soon.

Tom