Re: [RFA 2/8] Use counted_command_line everywhere

Pedro Alves
2018-04-24T16:43:08Z

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