From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5054 invoked by alias); 30 Jul 2009 06:24:01 -0000 Received: (qmail 5033 invoked by uid 22791); 30 Jul 2009 06:23:59 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_37,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 30 Jul 2009 06:23:50 +0000 Received: (qmail 21332 invoked from network); 30 Jul 2009 06:23:48 -0000 Received: from unknown (HELO wind.localnet) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 30 Jul 2009 06:23:48 -0000 From: Vladimir Prus To: tromey@redhat.com Subject: Re: [RFA] Implement -break-commands Date: Thu, 30 Jul 2009 07:54:00 -0000 User-Agent: KMail/1.11.90 (Linux/2.6.24-24-generic; KDE/4.2.90; i686; svn-979530; 2009-06-10) Cc: gdb-patches@sources.redhat.com References: <200907271303.13335.vladimir@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_xxTcK5LRF84wB+O" Message-Id: <200907301023.45893.vladimir@codesourcery.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-07/txt/msg00734.txt.bz2 --Boundary-00=_xxTcK5LRF84wB+O Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-length: 2411 On Monday 27 July 2009 Tom Tromey wrote: > >>>>> "Volodya" == Vladimir Prus 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 --Boundary-00=_xxTcK5LRF84wB+O Content-Type: text/x-patch; charset="UTF-8"; name="1.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="1.diff" Content-length: 7583 commit 51e2d6c76a2fa48b1eb0a4f7c9af87789fe45a26 Author: Vladimir Prus Date: Fri Jul 24 13:53:37 2009 +0400 Refactor reading of commands 2009-07-27 Jim Ingham Vladimir Prus * 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 **); --Boundary-00=_xxTcK5LRF84wB+O Content-Type: text/x-patch; charset="UTF-8"; name="2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="2.diff" Content-length: 11343 commit da96c898c51d51be3486026114752970b6092fb8 Author: Vladimir Prus Date: Sat Jul 25 18:12:39 2009 +0400 Implement -break-commands gdb/ 2009-07-27 Jim Ingham Vladimir Prus * 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 * gdb.texinfo (GDB/MI Breakpoint Commands): Document -break-commands. gdb/testsuite/ 2009-07-27 Vladimir Prus * 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 [ [...]]", 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 --Boundary-00=_xxTcK5LRF84wB+O--