[RFA 3/3] Change counted_command_line to a shared_ptr

[Tom Tromey <tom at tromey dot com>]

This changes counted_command_line to be a typedef for std::shared_ptr
and removes the associated cleanups.  In the long run I believe that
cmd_list_element should also be changed to use a shared_ptr.

ChangeLog
2017-08-30  Tom Tromey  <tom@tromey.com>

	* breakpoint.c (struct counted_command_line): Remove.
	(breakpoint_commands): Update.
	(alloc_counted_command_line, incref_counted_command_line)
	(decref_counted_command_line, do_cleanup_counted_command_line)
	(make_cleanup_decref_counted_command_line): Remove.
	(breakpoint_set_commands, commands_command_1, ~bpstats, bpstats)
	(bpstat_clear_actions, bpstat_do_actions_1, watchpoint_check)
	(bpstat_stop_status, print_one_breakpoint_location, ~breakpoint)
	(save_breakpoints): Update.
	* breakpoint.h (counted_command_line): Now a typedef to
	shared_ptr.
	(struct breakpoint) <commands>: Now a counted_command_line.
	(struct bpstats) <command>: Likewise.
---
 gdb/ChangeLog    |  16 +++++++
 gdb/breakpoint.c | 130 ++++++++-----------------------------------------------
 gdb/breakpoint.h |   9 ++--
 3 files changed, 39 insertions(+), 116 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 28de155..0816cdd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,21 @@
 2017-08-30  Tom Tromey  <tom@tromey.com>
 
+	* breakpoint.c (struct counted_command_line): Remove.
+	(breakpoint_commands): Update.
+	(alloc_counted_command_line, incref_counted_command_line)
+	(decref_counted_command_line, do_cleanup_counted_command_line)
+	(make_cleanup_decref_counted_command_line): Remove.
+	(breakpoint_set_commands, commands_command_1, ~bpstats, bpstats)
+	(bpstat_clear_actions, bpstat_do_actions_1, watchpoint_check)
+	(bpstat_stop_status, print_one_breakpoint_location, ~breakpoint)
+	(save_breakpoints): Update.
+	* breakpoint.h (counted_command_line): Now a typedef to
+	shared_ptr.
+	(struct breakpoint) <commands>: Now a counted_command_line.
+	(struct bpstats) <command>: Likewise.
+
+2017-08-30  Tom Tromey  <tom@tromey.com>
+
 	* breakpoint.c (struct commands_info, do_map_commands_command):
 	Remove.
 	(commands_command_1): Update.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index fd28f7c..72de69a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -358,21 +358,10 @@ static char *dprintf_channel;
    has disconnected.  */
 static int disconnected_dprintf = 1;
 
-/* A reference-counted struct command_line.  This lets multiple
-   breakpoints share a single command list.  */
-struct counted_command_line
-{
-  /* The reference count.  */
-  int refc;
-
-  /* The command list.  */
-  struct command_line *commands;
-};
-
 struct command_line *
 breakpoint_commands (struct breakpoint *b)
 {
-  return b->commands ? b->commands->commands : NULL;
+  return b->commands ? b->commands.get () : NULL;
 }
 
 /* Flag indicating that a command has proceeded the inferior past the
@@ -718,64 +707,6 @@ clear_breakpoint_hit_counts (void)
     b->hit_count = 0;
 }
 
-/* Allocate a new counted_command_line with reference count of 1.
-   The new structure owns COMMANDS.  */
-
-static struct counted_command_line *
-alloc_counted_command_line (struct command_line *commands)
-{
-  struct counted_command_line *result = XNEW (struct counted_command_line);
-
-  result->refc = 1;
-  result->commands = commands;
-
-  return result;
-}
-
-/* Increment reference count.  This does nothing if CMD is NULL.  */
-
-static void
-incref_counted_command_line (struct counted_command_line *cmd)
-{
-  if (cmd)
-    ++cmd->refc;
-}
-
-/* Decrement reference count.  If the reference count reaches 0,
-   destroy the counted_command_line.  Sets *CMDP to NULL.  This does
-   nothing if *CMDP is NULL.  */
-
-static void
-decref_counted_command_line (struct counted_command_line **cmdp)
-{
-  if (*cmdp)
-    {
-      if (--(*cmdp)->refc == 0)
-	{
-	  free_command_lines (&(*cmdp)->commands);
-	  xfree (*cmdp);
-	}
-      *cmdp = NULL;
-    }
-}
-
-/* A cleanup function that calls decref_counted_command_line.  */
-
-static void
-do_cleanup_counted_command_line (void *arg)
-{
-  decref_counted_command_line ((struct counted_command_line **) arg);
-}
-
-/* Create a cleanup that calls decref_counted_command_line on the
-   argument.  */
-
-static struct cleanup *
-make_cleanup_decref_counted_command_line (struct counted_command_line **cmdp)
-{
-  return make_cleanup (do_cleanup_counted_command_line, cmdp);
-}
-
 
 /* Return the breakpoint with the specified number, or NULL
    if the number does not refer to an existing breakpoint.  */
@@ -1304,8 +1235,7 @@ breakpoint_set_commands (struct breakpoint *b,
 {
   validate_commands_for_breakpoint (b, commands.get ());
 
-  decref_counted_command_line (&b->commands);
-  b->commands = alloc_counted_command_line (commands.release ());
+  b->commands = std::move (commands);
   observer_notify_breakpoint_modified (b);
 }
 
@@ -1361,12 +1291,7 @@ static void
 commands_command_1 (const char *arg, int from_tty,
 		    struct command_line *control)
 {
-  struct cleanup *cleanups;
-  struct counted_command_line *cmd = NULL;
-
-  /* If we read command lines from the user, then `info' will hold an
-     extra reference to the commands that we must clean up.  */
-  cleanups = make_cleanup_decref_counted_command_line (&cmd);
+  counted_command_line cmd;
 
   std::string new_arg;
 
@@ -1385,10 +1310,8 @@ commands_command_1 (const char *arg, int from_tty,
      {
        if (cmd == NULL)
 	 {
-	   command_line_up l;
-
 	   if (control != NULL)
-	     l = copy_command_lines (control->body_list[0]);
+	     cmd = copy_command_lines (control->body_list[0]);
 	   else
 	     {
 	       struct cleanup *old_chain;
@@ -1400,25 +1323,21 @@ commands_command_1 (const char *arg, int from_tty,
 
 	       old_chain = make_cleanup (xfree, str);
 
-	       l = read_command_lines (str,
-				       from_tty, 1,
-				       (is_tracepoint (b)
-					? check_tracepoint_command : 0),
-				       b);
+	       cmd = read_command_lines (str,
+					 from_tty, 1,
+					 (is_tracepoint (b)
+					  ? check_tracepoint_command : 0),
+					 b);
 
 	       do_cleanups (old_chain);
 	     }
-
-	   cmd = alloc_counted_command_line (l.release ());
 	 }
 
        /* If a breakpoint was on the list more than once, we don't need to
 	  do anything.  */
        if (b->commands != cmd)
 	 {
-	   validate_commands_for_breakpoint (b, cmd->commands);
-	   incref_counted_command_line (cmd);
-	   decref_counted_command_line (&b->commands);
+	   validate_commands_for_breakpoint (b, cmd.get ());
 	   b->commands = cmd;
 	   observer_notify_breakpoint_modified (b);
 	 }
@@ -1426,8 +1345,6 @@ commands_command_1 (const char *arg, int from_tty,
 
   if (cmd == NULL)
     error (_("No breakpoints specified."));
-
-  do_cleanups (cleanups);
 }
 
 static void
@@ -4428,7 +4345,6 @@ bpstats::~bpstats ()
 {
   if (old_val != NULL)
     value_free (old_val);
-  decref_counted_command_line (&commands);
   if (bp_location_at != NULL)
     decref_bp_location (&bp_location_at);
 }
@@ -4470,7 +4386,6 @@ bpstats::bpstats (const bpstats &other)
       release_value (old_val);
     }
   incref_bp_location (bp_location_at);
-  incref_counted_command_line (commands);
 }
 
 /* Return a copy of a bpstat.  Like "bs1 = bs2" but all storage that
@@ -4589,7 +4504,7 @@ bpstat_clear_actions (void)
 
   for (bs = tp->control.stop_bpstat; bs != NULL; bs = bs->next)
     {
-      decref_counted_command_line (&bs->commands);
+      bs->commands = NULL;
 
       if (bs->old_val != NULL)
 	{
@@ -4668,9 +4583,7 @@ bpstat_do_actions_1 (bpstat *bsp)
   breakpoint_proceeded = 0;
   for (; bs != NULL; bs = bs->next)
     {
-      struct counted_command_line *ccmd;
-      struct command_line *cmd;
-      struct cleanup *this_cmd_tree_chain;
+      struct command_line *cmd = NULL;
 
       /* Take ownership of the BSP's command tree, if it has one.
 
@@ -4682,10 +4595,10 @@ bpstat_do_actions_1 (bpstat *bsp)
          commands are only executed once, we don't need to copy it; we
          can clear the pointer in the bpstat, and make sure we free
          the tree when we're done.  */
-      ccmd = bs->commands;
+      counted_command_line ccmd = bs->commands;
       bs->commands = NULL;
-      this_cmd_tree_chain = make_cleanup_decref_counted_command_line (&ccmd);
-      cmd = ccmd ? ccmd->commands : NULL;
+      if (ccmd != NULL)
+	cmd = ccmd.get ();
       if (command_line_is_silent (cmd))
 	{
 	  /* The action has been already done by bpstat_stop_status.  */
@@ -4702,9 +4615,6 @@ bpstat_do_actions_1 (bpstat *bsp)
 	    cmd = cmd->next;
 	}
 
-      /* We can free this command tree now.  */
-      do_cleanups (this_cmd_tree_chain);
-
       if (breakpoint_proceeded)
 	{
 	  if (current_ui->async)
@@ -5240,7 +5150,7 @@ watchpoint_check (void *p)
 	}
 
       /* Make sure the watchpoint's commands aren't executed.  */
-      decref_counted_command_line (&b->commands);
+      b->commands = NULL;
       watchpoint_del_at_next_stop (b);
 
       return WP_DELETED;
@@ -5730,9 +5640,8 @@ bpstat_stop_status (struct address_space *aspace,
 	      if (b->silent)
 		bs->print = 0;
 	      bs->commands = b->commands;
-	      incref_counted_command_line (bs->commands);
 	      if (command_line_is_silent (bs->commands
-					  ? bs->commands->commands : NULL))
+					  ? bs->commands.get () : NULL))
 		bs->print = 0;
 
 	      b->ops->after_condition_true (bs);
@@ -6595,7 +6504,7 @@ print_one_breakpoint_location (struct breakpoint *b,
 	}
     }
 
-  l = b->commands ? b->commands->commands : NULL;
+  l = b->commands ? b->commands.get () : NULL;
   if (!part_of_multiple && l)
     {
       annotate_field (9);
@@ -12674,7 +12583,6 @@ static const struct bp_location_ops bp_location_ops =
 
 breakpoint::~breakpoint ()
 {
-  decref_counted_command_line (&this->commands);
   xfree (this->cond_string);
   xfree (this->extra_string);
   xfree (this->filter);
@@ -15454,7 +15362,7 @@ save_breakpoints (char *filename, int from_tty,
 	current_uiout->redirect (&fp);
 	TRY
 	  {
-	    print_command_lines (current_uiout, tp->commands->commands, 2);
+	    print_command_lines (current_uiout, tp->commands.get (), 2);
 	  }
 	CATCH (ex, RETURN_MASK_ALL)
 	  {
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index f3c0961..ae7547e 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -655,10 +655,9 @@ enum watchpoint_triggered
 typedef struct bp_location *bp_location_p;
 DEF_VEC_P(bp_location_p);
 
-/* A reference-counted struct command_line.  This lets multiple
-   breakpoints share a single command list.  This is an implementation
+/* A reference-counted struct command_line. This is an implementation
    detail to the breakpoints module.  */
-struct counted_command_line;
+typedef std::shared_ptr<command_line> counted_command_line;
 
 /* Some targets (e.g., embedded PowerPC) need two debug registers to set
    a watchpoint over a memory region.  If this flag is true, GDB will use
@@ -710,7 +709,7 @@ struct breakpoint
 
   /* Chain of command lines to execute when this breakpoint is
      hit.  */
-  counted_command_line *commands = NULL;
+  counted_command_line commands;
   /* Stack depth (address of frame).  If nonzero, break only if fp
      equals this.  */
   struct frame_id frame_id = null_frame_id;
@@ -1116,7 +1115,7 @@ struct bpstats
     struct breakpoint *breakpoint_at;
 
     /* The associated command list.  */
-    struct counted_command_line *commands;
+    counted_command_line commands;
 
     /* Old value associated with a watchpoint.  */
     struct value *old_val;
-- 
2.9.4