This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Implement -break-commands
On Monday 27 July 2009 Tom Tromey wrote:
> >>>>> "Volodya" == Vladimir Prus <vladimir@codesourcery.com> writes:
>
> This all looks reasonable to me. I have a few style nits.
>
> Volodya> -/* Read one line from the input stream. If the command is an "end",
> Volodya> - return such an indication to the caller. If PARSE_COMMANDS is true,
> Volodya> - strip leading whitespace (trailing whitespace is always stripped)
> Volodya> - in the line, attempt to recognize GDB control commands, and also
> Volodya> - return an indication if the command is an "else" or a nop.
> Volodya> - Otherwise, only "end" is recognized. */
>
> Volodya> -static enum misc_command_type
> Volodya> -read_next_line (struct command_line **command, int parse_commands)
> Volodya> +char *
> Volodya> +read_next_line ()
>
> This should be marked `static' (it is declared that way but it is
> clearer to mark the definition as well). I think it needs a header
> comment.
>
> Volodya> static enum command_control_type
> Volodya> -recurse_read_control_structure (struct command_line *current_cmd)
> Volodya> +recurse_read_control_structure (char * (*read_next_line_func) (),
> Volodya> + struct command_line *current_cmd)
>
> The header comment should be updated to mention the new argument.
> And if you don't mind, please fix the reference to the non-existing
> "parent_control" parameter.
>
> Volodya> +extern struct command_line *read_command_lines_1
> Volodya> +(char * (*read_next_line_func) (), int parse_commands);
>
> Indentation on the 2nd line.
>
> There's some other little style nits in the patch -- over-bracing in the
> second patch, mostly.
>
> Volodya> +void
> Volodya> +breakpoint_set_commands (struct breakpoint *b, struct command_line *commands)
>
> Needs a header comment.
>
> Volodya> +static char **mi_command_line_array;
> Volodya> +static int mi_command_line_array_cnt;
> Volodya> +static int mi_command_line_array_ptr;
>
> [...]
>
> Volodya> + break_command = read_command_lines_1 (mi_read_next_line, 0);
>
> I think this would be cleaner if read_command_lines_1 took a "user_data"
> argument and then there were no new globals.
This revision addresses all the comments above, except that request for 'closure' pointer to
read_command_lines_1 if addressed by a comment saying why we don't do that. Eli, I've
also applied your documentation suggestions. Is this OK?
- Volodya
commit 51e2d6c76a2fa48b1eb0a4f7c9af87789fe45a26
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Fri Jul 24 13:53:37 2009 +0400
Refactor reading of commands
2009-07-27 Jim Ingham <jingham@apple.com>
Vladimir Prus <vladimir@codesourcery.com>
* defs.h (read_command_lines_1): Declare.
* cli/cli-script.c (read_next_line): Only return string,
do not process.
(process_next_line): New, extracted from read_next_line.
(recurse_read_control_structure): Take a function pointer to the
read function.
(get_command_line) Pass the read_next_line as reader function
into recurse_read_control_structure.
(read_command_lines_1): New, extracted from...
(read_command_lines): ...here.
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 054ce90..62c4f24 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -39,12 +39,15 @@
/* Prototypes for local functions */
static enum command_control_type
- recurse_read_control_structure (struct command_line *current_cmd);
+recurse_read_control_structure (char * (*read_next_line_func) (),
+ struct command_line *current_cmd);
static char *insert_args (char *line);
static struct cleanup * setup_user_args (char *p);
+static char *read_next_line ();
+
/* Level of control structure when reading. */
static int control_level;
@@ -114,7 +117,7 @@ get_command_line (enum command_control_type type, char *arg)
old_chain = make_cleanup_free_command_lines (&cmd);
/* Read in the body of this command. */
- if (recurse_read_control_structure (cmd) == invalid_control)
+ if (recurse_read_control_structure (read_next_line, cmd) == invalid_control)
{
warning (_("Error reading in canned sequence of commands."));
do_cleanups (old_chain);
@@ -837,19 +840,12 @@ realloc_body_list (struct command_line *command, int new_length)
command->body_count = new_length;
}
-/* Read one line from the input stream. If the command is an "end",
- return such an indication to the caller. If PARSE_COMMANDS is true,
- strip leading whitespace (trailing whitespace is always stripped)
- in the line, attempt to recognize GDB control commands, and also
- return an indication if the command is an "else" or a nop.
- Otherwise, only "end" is recognized. */
-static enum misc_command_type
-read_next_line (struct command_line **command, int parse_commands)
+static char *
+read_next_line ()
{
- char *p, *p1, *prompt_ptr, control_prompt[256];
+ char *prompt_ptr, control_prompt[256];
int i = 0;
- int not_handled = 0;
if (control_level >= 254)
error (_("Control nesting too deep!"));
@@ -866,7 +862,21 @@ read_next_line (struct command_line **command, int parse_commands)
else
prompt_ptr = NULL;
- p = command_line_input (prompt_ptr, instream == stdin, "commands");
+ return command_line_input (prompt_ptr, instream == stdin, "commands");
+}
+
+/* Process one input line. If the command is an "end",
+ return such an indication to the caller. If PARSE_COMMANDS is true,
+ strip leading whitespace (trailing whitespace is always stripped)
+ in the line, attempt to recognize GDB control commands, and also
+ return an indication if the command is an "else" or a nop.
+ Otherwise, only "end" is recognized. */
+
+static enum misc_command_type
+process_next_line (char *p, struct command_line **command, int parse_commands)
+{
+ char *p1;
+ int not_handled = 0;
/* Not sure what to do here. */
if (p == NULL)
@@ -973,18 +983,20 @@ read_next_line (struct command_line **command, int parse_commands)
}
/* Recursively read in the control structures and create a command_line
- structure from them.
+ structure from them. Use read_next_line_func to obtain lines of
+ the command.
- The parent_control parameter is the control structure in which the
- following commands are nested. */
+*/
static enum command_control_type
-recurse_read_control_structure (struct command_line *current_cmd)
+recurse_read_control_structure (char * (*read_next_line_func) (),
+ struct command_line *current_cmd)
{
int current_body, i;
enum misc_command_type val;
enum command_control_type ret;
struct command_line **body_ptr, *child_tail, *next;
+ char *p;
child_tail = NULL;
current_body = 1;
@@ -1002,7 +1014,8 @@ recurse_read_control_structure (struct command_line *current_cmd)
dont_repeat ();
next = NULL;
- val = read_next_line (&next, current_cmd->control_type != python_control);
+ val = process_next_line (read_next_line_func (), &next,
+ current_cmd->control_type != python_control);
/* Just skip blanks and comments. */
if (val == nop_command)
@@ -1068,7 +1081,7 @@ recurse_read_control_structure (struct command_line *current_cmd)
|| next->control_type == commands_control)
{
control_level++;
- ret = recurse_read_control_structure (next);
+ ret = recurse_read_control_structure (read_next_line_func, next);
control_level--;
if (ret != simple_control)
@@ -1095,12 +1108,7 @@ recurse_read_control_structure (struct command_line *current_cmd)
struct command_line *
read_command_lines (char *prompt_arg, int from_tty, int parse_commands)
{
- struct command_line *head, *tail, *next;
- struct cleanup *old_chain;
- enum command_control_type ret;
- enum misc_command_type val;
-
- control_level = 0;
+ struct command_line *head;
if (from_tty && input_from_terminal_p ())
{
@@ -1116,13 +1124,34 @@ read_command_lines (char *prompt_arg, int from_tty, int parse_commands)
}
}
+ head = read_command_lines_1 (read_next_line, parse_commands);
+
+ if (deprecated_readline_end_hook && from_tty && input_from_terminal_p ())
+ {
+ (*deprecated_readline_end_hook) ();
+ }
+ return (head);
+}
+
+/* Act the same way as read_command_lines, except that each new line is
+ obtained using READ_NEXT_LINE_FUNC. */
+
+struct command_line *
+read_command_lines_1 (char * (*read_next_line_func) (), int parse_commands)
+{
+ struct command_line *head, *tail, *next;
+ struct cleanup *old_chain;
+ enum command_control_type ret;
+ enum misc_command_type val;
+
+ control_level = 0;
head = tail = NULL;
old_chain = NULL;
while (1)
{
dont_repeat ();
- val = read_next_line (&next, parse_commands);
+ val = process_next_line (read_next_line_func (), &next, parse_commands);
/* Ignore blank lines or comments. */
if (val == nop_command)
@@ -1146,7 +1175,7 @@ read_command_lines (char *prompt_arg, int from_tty, int parse_commands)
|| next->control_type == commands_control)
{
control_level++;
- ret = recurse_read_control_structure (next);
+ ret = recurse_read_control_structure (read_next_line_func, next);
control_level--;
if (ret == invalid_control)
@@ -1177,11 +1206,7 @@ read_command_lines (char *prompt_arg, int from_tty, int parse_commands)
do_cleanups (old_chain);
}
- if (deprecated_readline_end_hook && from_tty && input_from_terminal_p ())
- {
- (*deprecated_readline_end_hook) ();
- }
- return (head);
+ return head;
}
/* Free a chain of struct command_line's. */
diff --git a/gdb/defs.h b/gdb/defs.h
index 6dc5a6c..f5127fd 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -696,6 +696,7 @@ struct command_line
};
extern struct command_line *read_command_lines (char *, int, int);
+extern struct command_line *read_command_lines_1 (char * (*) (), int);
extern void free_command_lines (struct command_line **);
commit da96c898c51d51be3486026114752970b6092fb8
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Sat Jul 25 18:12:39 2009 +0400
Implement -break-commands
gdb/
2009-07-27 Jim Ingham <jingham@apple.com>
Vladimir Prus <vladimir@codesourcery.com>
* breakpoint.c (get_breakpoint, breakpoint_set_commands): New.
(commands_command): Use breakpoint_set_commands.
* breakpoint.h (get_breakpoint, breakpoint_set_commands): Declare.
* mi/mi-cmds.h (mi_cmd_break_commands): New.
* mi/mi-cmds.c: Register -break-commands.
* mi/mi-cmd-break.c (mi_cmd_break_commands, mi_read_next_line)
(mi_command_line_array, mi_command_line_array_cnt)
(mi_command_line_array_ptr): New.
gdb/doc/
2009-07-27 Vladimir Prus <vladimir@codesourcery.com>
* gdb.texinfo (GDB/MI Breakpoint Commands): Document
-break-commands.
gdb/testsuite/
2009-07-27 Vladimir Prus <vladimir@codesourcery.com>
* lib/mi-support.exp (mi_list_breakpoints): Make it work.
* gdb.mi/mi-break.exp (test_breakpoint_commands): New.
Call it.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3a18c8f..4b9b44e 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -559,6 +559,20 @@ get_number_or_range (char **pp)
return last_retval;
}
+/* Return the breakpoint with the specified number, or NULL
+ if the number does not refer to an existing breakpoint. */
+
+struct breakpoint *
+get_breakpoint (int num)
+{
+ struct breakpoint *b;
+
+ ALL_BREAKPOINTS (b)
+ if (b->number == num)
+ return b;
+
+ return NULL;
+}
/* condition N EXP -- set break condition of breakpoint N to EXP. */
@@ -623,6 +637,17 @@ condition_command (char *arg, int from_tty)
error (_("No breakpoint number %d."), bnum);
}
+/* Set the command list of B to COMMANDS. */
+
+void
+breakpoint_set_commands (struct breakpoint *b, struct command_line *commands)
+{
+ free_command_lines (&b->commands);
+ b->commands = commands;
+ breakpoints_changed ();
+ observer_notify_breakpoint_modified (b->number);
+}
+
static void
commands_command (char *arg, int from_tty)
{
@@ -652,10 +677,7 @@ commands_command (char *arg, int from_tty)
struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
l = read_command_lines (tmpbuf, from_tty, 1);
do_cleanups (cleanups);
- free_command_lines (&b->commands);
- b->commands = l;
- breakpoints_changed ();
- observer_notify_breakpoint_modified (b->number);
+ breakpoint_set_commands (b, l);
return;
}
error (_("No breakpoint number %d."), bnum);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 6731e68..54e2ea0 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -821,6 +821,8 @@ extern int get_number (char **);
extern int get_number_or_range (char **);
+extern struct breakpoint *get_breakpoint (int num);
+
/* The following are for displays, which aren't really breakpoints, but
here is as good a place as any for them. */
@@ -836,6 +838,9 @@ extern void disable_breakpoint (struct breakpoint *);
extern void enable_breakpoint (struct breakpoint *);
+extern void breakpoint_set_commands (struct breakpoint *b,
+ struct command_line *commands);
+
/* Clear the "inserted" flag in all breakpoints. */
extern void mark_breakpoints_out (void);
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c3693fa..f598ebe 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21465,11 +21465,41 @@ line="5",times="0",ignore="3"@}]@}
@ignore
@subheading The @code{-break-catch} Command
@findex -break-catch
+@end ignore
@subheading The @code{-break-commands} Command
@findex -break-commands
-@end ignore
+@subsubheading Synopsis
+
+@smallexample
+ -break-commands @var{number} [ @var{command1} ... @var{commandN} ]
+@end smallexample
+
+Specifies the CLI commands that should be executed when breakpoint
+@var{number} is hit. The parameters @var{command1} to @var{commandN}
+are the commands. If no command is specified, any previously-set
+commands are cleared. @xref{Break Commands}. Typical use of this
+functionality is tracing a program, that is, printing of values of
+some variables whenever breakpoint is hit and then continuing.
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{commands}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-break-insert main
+^done,bkpt=@{number="1",type="breakpoint",disp="keep",
+enabled="y",addr="0x000100d0",func="main",file="hello.c",
+fullname="/home/foo/hello.c",line="5",times="0"@}
+(gdb)
+-break-commands 1 "print v" "continue"
+^done
+(gdb)
+@end smallexample
@subheading The @code{-break-condition} Command
@findex -break-condition
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 445c53e..9ab8f2d 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -252,3 +252,53 @@ mi_cmd_break_watch (char *command, char **argv, int argc)
error (_("mi_cmd_break_watch: Unknown watchpoint type."));
}
}
+
+/* The mi_read_next_line consults these variable to return successive
+ command lines. While it would be clearer to use a closure pointer,
+ it is not expected that any future code will use read_command_lines_1,
+ therefore no point of overengineering. */
+
+static char **mi_command_line_array;
+static int mi_command_line_array_cnt;
+static int mi_command_line_array_ptr;
+
+static char *
+mi_read_next_line ()
+{
+ if (mi_command_line_array_ptr == mi_command_line_array_cnt)
+ return NULL;
+ else
+ return mi_command_line_array[mi_command_line_array_ptr++];
+}
+
+void
+mi_cmd_break_commands (char *command, char **argv, int argc)
+{
+ struct command_line *break_command;
+ char *endptr;
+ int bnum;
+ struct breakpoint *b;
+
+ if (argc < 1)
+ error ("USAGE: %s <BKPT> [<COMMAND> [<COMMAND>...]]", command);
+
+ bnum = strtol (argv[0], &endptr, 0);
+ if (endptr == argv[0])
+ error ("breakpoint number argument \"%s\" is not a number.",
+ argv[0]);
+ else if (*endptr != '\0')
+ error ("junk at the end of breakpoint number argument \"%s\".",
+ argv[0]);
+
+ b = get_breakpoint (bnum);
+ if (b == NULL)
+ error ("breakpoint %d not found.", bnum);
+
+ mi_command_line_array = argv;
+ mi_command_line_array_ptr = 1;
+ mi_command_line_array_cnt = argc;
+
+ break_command = read_command_lines_1 (mi_read_next_line, 0);
+ breakpoint_set_commands (b, break_command);
+}
+
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 4911146..dd3d803 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -35,6 +35,7 @@ struct mi_cmd mi_cmds[] =
{
{ "break-after", { "ignore", 1 }, NULL },
{ "break-condition", { "cond", 1 }, NULL },
+ { "break-commands", { NULL, 0 }, mi_cmd_break_commands },
{ "break-delete", { "delete breakpoint", 1 }, NULL },
{ "break-disable", { "disable breakpoint", 1 }, NULL },
{ "break-enable", { "enable breakpoint", 1 }, NULL },
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index afcba1e..85ad0c4 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -37,6 +37,7 @@ typedef void (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
/* Function implementing each command */
extern mi_cmd_argv_ftype mi_cmd_break_insert;
+extern mi_cmd_argv_ftype mi_cmd_break_commands;
extern mi_cmd_argv_ftype mi_cmd_break_watch;
extern mi_cmd_argv_ftype mi_cmd_disassemble;
extern mi_cmd_argv_ftype mi_cmd_data_evaluate_expression;
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 4b8d101..619727d 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -197,6 +197,30 @@ proc test_disabled_creation {} {
"test disabled creation: cleanup"
}
+proc test_breakpoint_commands {} {
+ global line_callee2_body
+ global hex
+ global fullname
+
+ mi_create_breakpoint "basics.c:callee2" 7 keep callee2 ".*basics.c" $line_callee2_body $hex \
+ "breakpoint commands: insert breakpoint at basics.c:callee2"
+
+ mi_gdb_test "-break-commands 7 \"print 10\" \"continue\"" \
+ "\\^done" \
+ "breakpoint commands: set commands"
+
+ mi_gdb_test "-break-info 7" \
+ "\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"7\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee2\",file=\".*basics.c\",${fullname},line=\"$line_callee2_body\",times=\"0\",script=\{\"print 10\",\"continue\"\},original-location=\".*\"\}.*\\\]\}" \
+ "breakpoint commands: check that commands are set"
+
+ mi_gdb_test "-break-commands 7" \
+ "\\^done" \
+ "breakpoint commands: clear commands"
+
+ mi_list_breakpoints [list [list 7 "keep" "callee2" "basics.c" "$line_callee2_body" $hex]] \
+ "breakpoint commands: check that commands are cleared"
+}
+
test_tbreak_creation_and_listing
test_rbreak_creation_and_listing
@@ -206,5 +230,7 @@ test_error
test_disabled_creation
+test_breakpoint_commands
+
mi_gdb_exit
return 0
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 9b4c464..e691232 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1170,21 +1170,22 @@ proc mi_list_breakpoints { expected test } {
set body ""
set first 1
- foreach item $children {
+ foreach item $expected {
if {$first == 0} {
set body "$body,"
+ set first 0
}
- set number disp func file line address
set number [lindex $item 0]
set disp [lindex $item 1]
set func [lindex $item 2]
- set line [lindex $item 3]
- set address [lindex $item 4]
- set body "$body,bkpt=\{number=\"$number\",type=\"breakpoint\",disp=\"$disp\",enabled=\"y\",addr=\"$address\",func=\"$func\",file=\"$file\",${fullname},line=\"$line\",times=\"0\",original-location=\".*\"\}"
+ set file [lindex $item 3]
+ set line [lindex $item 4]
+ set address [lindex $item 5]
+ set body "${body}bkpt=\{number=\"$number\",type=\"breakpoint\",disp=\"$disp\",enabled=\"y\",addr=\"$address\",func=\"$func\",file=\".*$file\",${fullname},line=\"$line\",times=\"0\",original-location=\".*\"\}"
set first 0
}
- verbose -log "Expecint: 666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[$body\\\]\}" \
+ verbose -log "Expecting: 666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[$body\\\]\}"
mi_gdb_test "666-break-list" \
"666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[$body\\\]\}" \
$test