cli-script: use unique_ptr to not leak next struct

In cli/cli-script.c, process_next_line() allocates memory
which will eventually end up being assigned to the 'next'
field in struct command_line.  However, in a case
recurse_read_control_structure returns 'invalid_control'
this memory is leaked. This commit uses std::unique_ptr
as appropriate to prevent this leakage.

This issue was found by coverity scanning.

gdb/ChangeLog:

        * cli/cli-script.h (command_line_up): New unique_ptr typedef.
	* cli/cli-script.c (multi_line_command_p): Use unique_ptr
        command_line_up instead of struct command_line.
	(build_command_line): Likewise.
	(get_command_line): Update the cmd function call parameter.
	(process_next_line):  Use unique_ptr command_line_up instead
        of struct command_line.
	(recurse_read_control_structure): Change the the type of
        next to command_line_up.
	(read_command_lines_1): Change type of `next' to be
        command_line_up and update all references of `next'
        accordingly.
This commit is contained in:
Alexandra Hájková
2021-05-20 20:55:35 +02:00
parent 9a01ec4c03
commit bb6203bf1d
3 changed files with 55 additions and 36 deletions

View File

@ -1,3 +1,19 @@
2021-05-20 Alexandra Hájková <ahajkova@redhat.com>
Pedro Alves <pedro@palves.net>
* cli/cli-script.h (command_line_up): New unique_ptr typedef.
* cli/cli-script.c (multi_line_command_p): Use unique_ptr
command_line_up instead of struct command_line.
(build_command_line): Likewise.
(get_command_line): Update the cmd function call parameter.
(process_next_line): Use unique_ptr command_line_up instead
of struct command_line.
(recurse_read_control_structure): Change the the type of
next to command_line_up.
(read_command_lines_1): Change type of `next' to be
command_line_up and update all references of `next'
accordingly.
2021-05-20 Alexandra Hájková <ahajkova@redhat.com> 2021-05-20 Alexandra Hájková <ahajkova@redhat.com>
* MAINTAINERS (Write After Approval): Add myself. * MAINTAINERS (Write After Approval): Add myself.

View File

@ -155,7 +155,7 @@ multi_line_command_p (enum command_control_type type)
/* Allocate, initialize a new command line structure for one of the /* Allocate, initialize a new command line structure for one of the
control commands (if/while). */ control commands (if/while). */
static struct command_line * static command_line_up
build_command_line (enum command_control_type type, const char *args) build_command_line (enum command_control_type type, const char *args)
{ {
if (args == NULL || *args == '\0') if (args == NULL || *args == '\0')
@ -171,7 +171,7 @@ build_command_line (enum command_control_type type, const char *args)
} }
gdb_assert (args != NULL); gdb_assert (args != NULL);
return new struct command_line (type, xstrdup (args)); return command_line_up (new command_line (type, xstrdup (args)));
} }
/* Build and return a new command structure for the control commands /* Build and return a new command structure for the control commands
@ -181,7 +181,7 @@ counted_command_line
get_command_line (enum command_control_type type, const char *arg) get_command_line (enum command_control_type type, const char *arg)
{ {
/* Allocate and build a new command line structure. */ /* Allocate and build a new command line structure. */
counted_command_line cmd (build_command_line (type, arg), counted_command_line cmd (build_command_line (type, arg).release (),
command_lines_deleter ()); command_lines_deleter ());
/* Read in the body of this command. */ /* Read in the body of this command. */
@ -957,7 +957,7 @@ line_first_arg (const char *p)
Otherwise, only "end" is recognized. */ Otherwise, only "end" is recognized. */
static enum misc_command_type static enum misc_command_type
process_next_line (const char *p, struct command_line **command, process_next_line (const char *p, command_line_up *command,
int parse_commands, int parse_commands,
gdb::function_view<void (const char *)> validator) gdb::function_view<void (const char *)> validator)
@ -1055,9 +1055,9 @@ process_next_line (const char *p, struct command_line **command,
*command = build_command_line (guile_control, ""); *command = build_command_line (guile_control, "");
} }
else if (p_end - p == 10 && startswith (p, "loop_break")) else if (p_end - p == 10 && startswith (p, "loop_break"))
*command = new struct command_line (break_control); *command = command_line_up (new command_line (break_control));
else if (p_end - p == 13 && startswith (p, "loop_continue")) else if (p_end - p == 13 && startswith (p, "loop_continue"))
*command = new struct command_line (continue_control); *command = command_line_up (new command_line (continue_control));
else else
not_handled = 1; not_handled = 1;
} }
@ -1065,22 +1065,12 @@ process_next_line (const char *p, struct command_line **command,
if (!parse_commands || not_handled) if (!parse_commands || not_handled)
{ {
/* A normal command. */ /* A normal command. */
*command = new struct command_line (simple_control, *command = command_line_up (new command_line (simple_control,
savestring (p, p_end - p)); savestring (p, p_end - p)));
} }
if (validator) if (validator)
{ validator ((*command)->line);
try
{
validator ((*command)->line);
}
catch (const gdb_exception &ex)
{
free_command_lines (command);
throw;
}
}
/* Nothing special. */ /* Nothing special. */
return ok_command; return ok_command;
@ -1097,10 +1087,11 @@ recurse_read_control_structure (gdb::function_view<const char * ()> read_next_li
{ {
enum misc_command_type val; enum misc_command_type val;
enum command_control_type ret; enum command_control_type ret;
struct command_line *child_tail, *next; struct command_line *child_tail;
counted_command_line *current_body = &current_cmd->body_list_0; counted_command_line *current_body = &current_cmd->body_list_0;
command_line_up next;
child_tail = NULL; child_tail = nullptr;
/* Sanity checks. */ /* Sanity checks. */
if (current_cmd->control_type == simple_control) if (current_cmd->control_type == simple_control)
@ -1111,8 +1102,8 @@ recurse_read_control_structure (gdb::function_view<const char * ()> read_next_li
{ {
dont_repeat (); dont_repeat ();
next = NULL; next = nullptr;
val = process_next_line (read_next_line_func (), &next, val = process_next_line (read_next_line_func (), &next,
current_cmd->control_type != python_control current_cmd->control_type != python_control
&& current_cmd->control_type != guile_control && current_cmd->control_type != guile_control
&& current_cmd->control_type != compile_control, && current_cmd->control_type != compile_control,
@ -1144,7 +1135,7 @@ recurse_read_control_structure (gdb::function_view<const char * ()> read_next_li
&& current_body == &current_cmd->body_list_0) && current_body == &current_cmd->body_list_0)
{ {
current_body = &current_cmd->body_list_1; current_body = &current_cmd->body_list_1;
child_tail = NULL; child_tail = nullptr;
continue; continue;
} }
else else
@ -1154,21 +1145,26 @@ recurse_read_control_structure (gdb::function_view<const char * ()> read_next_li
} }
} }
if (child_tail) /* Transfer ownership of NEXT to the command's body list. */
if (child_tail != nullptr)
{ {
child_tail->next = next; child_tail->next = next.release ();
child_tail = child_tail->next;
} }
else else
*current_body = counted_command_line (next, command_lines_deleter ()); {
child_tail = next.get ();
child_tail = next; *current_body = counted_command_line (next.release (),
command_lines_deleter ());
}
/* If the latest line is another control structure, then recurse /* If the latest line is another control structure, then recurse
on it. */ on it. */
if (multi_line_command_p (next->control_type)) if (multi_line_command_p (child_tail->control_type))
{ {
control_level++; control_level++;
ret = recurse_read_control_structure (read_next_line_func, next, ret = recurse_read_control_structure (read_next_line_func,
child_tail,
validator); validator);
control_level--; control_level--;
@ -1240,10 +1236,11 @@ read_command_lines_1 (gdb::function_view<const char * ()> read_next_line_func,
int parse_commands, int parse_commands,
gdb::function_view<void (const char *)> validator) gdb::function_view<void (const char *)> validator)
{ {
struct command_line *tail, *next; struct command_line *tail;
counted_command_line head (nullptr, command_lines_deleter ()); counted_command_line head (nullptr, command_lines_deleter ());
enum command_control_type ret; enum command_control_type ret;
enum misc_command_type val; enum misc_command_type val;
command_line_up next;
control_level = 0; control_level = 0;
tail = NULL; tail = NULL;
@ -1273,7 +1270,7 @@ read_command_lines_1 (gdb::function_view<const char * ()> read_next_line_func,
if (multi_line_command_p (next->control_type)) if (multi_line_command_p (next->control_type))
{ {
control_level++; control_level++;
ret = recurse_read_control_structure (read_next_line_func, next, ret = recurse_read_control_structure (read_next_line_func, next.get (),
validator); validator);
control_level--; control_level--;
@ -1281,15 +1278,18 @@ read_command_lines_1 (gdb::function_view<const char * ()> read_next_line_func,
break; break;
} }
/* Transfer ownership of NEXT to the HEAD list. */
if (tail) if (tail)
{ {
tail->next = next; tail->next = next.release ();
tail = tail->next;
} }
else else
{ {
head = counted_command_line (next, command_lines_deleter ()); tail = next.get ();
head = counted_command_line (next.release (),
command_lines_deleter ());
} }
tail = next;
} }
dont_repeat (); dont_repeat ();

View File

@ -66,6 +66,9 @@ struct command_lines_deleter
/* A reference-counted struct command_line. */ /* A reference-counted struct command_line. */
typedef std::shared_ptr<command_line> counted_command_line; typedef std::shared_ptr<command_line> counted_command_line;
/* A unique_ptr specialization for command_line. */
typedef std::unique_ptr<command_line, command_lines_deleter> command_line_up;
/* * Structure for saved commands lines (for breakpoints, defined /* * Structure for saved commands lines (for breakpoints, defined
commands, etc). */ commands, etc). */