Re: [RFA 2/8] Use counted_command_line everywhere

[Pedro Alves <palves@redhat.com>]

On 04/19/2018 08:15 PM, Tom Tromey wrote:
> Currently command lines are reference counted using shared_ptr only
> when attached to breakpoints.  This patch changes gdb to use
> shared_ptr in commands as well.  This allows for the removal of
> copy_command_lines.
> 
> Note that the change to execute_user_command explicitly makes a new
> reference to the command line.  This will be used in a later patch.
> 
> This simplifies struct command_line based on the observation that a
> given command can have at most two child bodies: an "if" can have both
> "then" and "else" parts.  Perhaps the names I've chosen for the
> replacements here are not very good -- your input requested.
> 

Names look fine to me.

> @@ -2663,11 +2663,10 @@ trace_dump_actions (struct command_line *action,
>  
>        if (cmd_cfunc_eq (cmd, while_stepping_pseudocommand))
>  	{
> -	  int i;
> -
> -	  for (i = 0; i < action->body_count; ++i)
> -	    trace_dump_actions (action->body_list[i],
> -				1, stepping_frame, from_tty);
> +	  trace_dump_actions (action->body_list_0.get (),
> +			      1, stepping_frame, from_tty);
> +	  trace_dump_actions (action->body_list_1.get (),
> +			      1, stepping_frame, from_tty);

Hmm, this looked suspicious.  I'm not seeing why would a
while-stepping action have two body lists.  I guess this
happens to work because trace_dump_actions does nothing
if ACTION is NULL.

> @@ -2797,17 +2792,13 @@ all_tracepoint_actions_and_cleanup (struct breakpoint *t)
>    if (*default_collect)
>      {
>        struct command_line *default_collect_action;
> -      char *default_collect_line;
> -
> -      default_collect_line = xstrprintf ("collect %s", default_collect);
> -      make_cleanup (xfree, default_collect_line);
> +      gdb::unique_xmalloc_ptr<char> default_collect_line
> +	(xstrprintf ("collect %s", default_collect));

string_printf ?

>  
> -      validate_actionline (default_collect_line, t);
> -      default_collect_action = XNEW (struct command_line);
> -      make_cleanup (xfree, default_collect_action);
> -      default_collect_action->next = actions;
> -      default_collect_action->line = default_collect_line;
> -      actions = default_collect_action;
> +      validate_actionline (default_collect_line.get (), t);
> +      actions.reset (new struct command_line (simple_control,
> +					      default_collect_line.release ()),
> +		     command_lines_deleter ());
>      }

LGTM.

It's a shame that we can't use std::make_shared to avoid
the separate control block allocation.

Thanks,
Pedro Alves