[RFA] Implement -break-commands

[RFA] Implement -break-commands

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 801 bytes --]


The below patches implement MI -break-commands. As the name implies,
it's the same as CLI 'commands'. There are two patches: one
refactors CLI 'commands' for easier reuse, and the second actually
adds -break-commands. The patches mostly were derived from Apple
branch, and then adjusted for CVS HEAD. One functional change I've
made is that '-break-commands <number>' does not report the list
of currently set commands -- since -break-info already prints his
information, it does not seem necessary to add another way. I've
also added a test and docs.

The parts of this patch that are not MI are:
1. Slight refactoring of cli/cli-script.c
2. Slight refactoring of breakpoint.c

I'd appreciate if somebody could review them.

Eli, there's also doc change here, comments appreciated.

Thanks,
Volodya

[-- Attachment #2: 1.diff --]
[-- Type: text/x-patch, Size: 7345 bytes --]

commit 62615d166f3452572e17644d854285040d86bcdb
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..5478145 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)
+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)
@@ -979,12 +989,14 @@ read_next_line (struct command_line **command, int parse_commands)
    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..c1f8b60 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -696,6 +696,8 @@ struct command_line
   };
 
 extern struct command_line *read_command_lines (char *, int, int);
+extern struct command_line *read_command_lines_1 
+(char * (*read_next_line_func) (), int parse_commands);
 
 extern void free_command_lines (struct command_line **);
 

[-- Attachment #3: 2.diff --]
[-- Type: text/x-patch, Size: 10940 bytes --]

commit b3ee1c6c1c6c179c7ee652b5c591a5b07b80c46e
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..bb8c082 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;
+}
 
 \f
 /* condition N EXP -- set break condition of breakpoint N to EXP.  */
@@ -623,6 +637,16 @@ condition_command (char *arg, int from_tty)
   error (_("No breakpoint number %d."), bnum);
 }
 
+
+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 +676,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..df0ffb6 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21465,11 +21465,39 @@ 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}
+
+@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-command "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..bb7ea14 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -252,3 +252,57 @@ mi_cmd_break_watch (char *command, char **argv, int argc)
       error (_("mi_cmd_break_watch: Unknown watchpoint type."));
     }
 }
+
+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

RE: [RFA] Implement -break-commands

[Marc Khouzam]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Vladimir Prus
> Sent: Monday, July 27, 2009 5:03 AM
> To: gdb-patches@sources.redhat.com
> Subject: [RFA] Implement -break-commands
> 
> 
> The below patches implement MI -break-commands. As the name implies,
> it's the same as CLI 'commands'. There are two patches: one
> refactors CLI 'commands' for easier reuse, and the second actually
> adds -break-commands. The patches mostly were derived from Apple
> branch, and then adjusted for CVS HEAD. One functional change I've
> made is that '-break-commands <number>' does not report the list
> of currently set commands -- since -break-info already prints his
> information, it does not seem necessary to add another way. I've
> also added a test and docs.

Nice addition.

> 
> The parts of this patch that are not MI are:
> 1. Slight refactoring of cli/cli-script.c
> 2. Slight refactoring of breakpoint.c
> 
> I'd appreciate if somebody could review them.
> 
> Eli, there's also doc change here, comments appreciated.

I noticed the example in the doc is missing the breakpoint number:

	> +-break-command "print v" "continue"

Unless it is optional?

Marc

Re: [RFA] Implement -break-commands

[Vladimir Prus]

On Monday 27 July 2009 Marc Khouzam wrote:

> > -----Original Message-----
> > From: gdb-patches-owner@sourceware.org 
> > [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Vladimir Prus
> > Sent: Monday, July 27, 2009 5:03 AM
> > To: gdb-patches@sources.redhat.com
> > Subject: [RFA] Implement -break-commands
> > 
> > 
> > The below patches implement MI -break-commands. As the name implies,
> > it's the same as CLI 'commands'. There are two patches: one
> > refactors CLI 'commands' for easier reuse, and the second actually
> > adds -break-commands. The patches mostly were derived from Apple
> > branch, and then adjusted for CVS HEAD. One functional change I've
> > made is that '-break-commands <number>' does not report the list
> > of currently set commands -- since -break-info already prints his
> > information, it does not seem necessary to add another way. I've
> > also added a test and docs.
> 
> Nice addition.
> 
> > 
> > The parts of this patch that are not MI are:
> > 1. Slight refactoring of cli/cli-script.c
> > 2. Slight refactoring of breakpoint.c
> > 
> > I'd appreciate if somebody could review them.
> > 
> > Eli, there's also doc change here, comments appreciated.
> 
> I noticed the example in the doc is missing the breakpoint number:
> 
> 	> +-break-command "print v" "continue"
> 
> Unless it is optional?

It's a doc bug -- thanks for noticing. I've fixed it in my patch tree.

- Volodya

Re: [RFA] Implement -break-commands

[Vladimir Prus]

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.

Is that something you want fixed? I do prefer bracing everything, since that
is safer in the long run.

> 
> Volodya> +void
> Volodya> +breakpoint_set_commands (struct breakpoint *b, struct command_line *commands)
> 
> Needs a header comment.

Do we have a policy if function should be documented in header, or implementation? It
seems many existing files have comments on implementation. 

> 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.

It would be clearer, though I decided not to do that because it's not
likely we'll need another such function, and the use of globals in this
context is ugly, but safe. Shall I refactor?

Thanks,
Volodya

Re: [RFA] Implement -break-commands

[Tom Tromey]

>>>>> "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.

Tom

Re: [RFA] Implement -break-commands

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Mon, 27 Jul 2009 13:03:12 +0400
> 
> The below patches implement MI -break-commands. As the name implies,
> it's the same as CLI 'commands'.

Thanks.  Could you perhaps say a few words about the use-cases where
this is needed by a front end?

> +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}

Two spaces between sentences, please.

> +-break-command "print v" "continue"

"break-command" or "break-commands"?

OK with those changes.

How about a NEWS entry for this?

Re: [RFA] Implement -break-commands

[Tom Tromey]

Tom> There's some other little style nits in the patch -- over-bracing
Tom> in the second patch, mostly.

Volodya> Is that something you want fixed? I do prefer bracing
Volodya> everything, since that is safer in the long run.

Yes, please fix this.  AFAIK this is the documented style.

Volodya> +void
Volodya> +breakpoint_set_commands (struct breakpoint *b, struct command_line *commands)

Tom> Needs a header comment.

Volodya> Do we have a policy if function should be documented in header,
Volodya> or implementation? It seems many existing files have comments
Volodya> on implementation.

It is hard to say whether we arrived at a policy the last time we had
this discussion.  These things rarely seem to converge, they just wander
off into the weeds.

I usually follow the prevailing style of the module I am modifying.
That is always a safe thing to do.

Tom> I think this would be cleaner if read_command_lines_1 took a "user_data"
Tom> argument and then there were no new globals.

Volodya> It would be clearer, though I decided not to do that because
Volodya> it's not likely we'll need another such function, and the use
Volodya> of globals in this context is ugly, but safe. Shall I refactor?

This one is up to you :-)

Tom

Re: [RFA] Implement -break-commands

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 2411 bytes --]

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


[-- Attachment #2: 1.diff --]
[-- Type: text/x-patch, Size: 7583 bytes --]

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 **);
 

[-- Attachment #3: 2.diff --]
[-- Type: text/x-patch, Size: 11343 bytes --]

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;
+}
 
 \f
 /* 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

Re: [RFA] Implement -break-commands

[Tom Tromey]

>>>>> "Volodya" == Vladimir Prus <vladimir@codesourcery.com> writes:

Volodya> This revision addresses all the comments above, except that
Volodya> request for 'closure' pointer to read_command_lines_1 if
Volodya> addressed by a comment saying why we don't do that.

There still isn't a header comment on read_next_line.  Just something
like "helper function for use with recurse_read_control_structure" would
be fine.  This is ok with the addition of a comment.

thanks,
Tom

Re: [RFA] Implement -break-commands

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Thu, 30 Jul 2009 10:23:45 +0400
> Cc: gdb-patches@sources.redhat.com
> 
> Eli, I've also applied your documentation suggestions. Is this OK?

Yes, thanks.

Re: [RFA] Implement -break-commands

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 588 bytes --]

On Thursday 30 July 2009 Tom Tromey wrote:

> >>>>> "Volodya" == Vladimir Prus <vladimir@codesourcery.com> writes:
> 
> Volodya> This revision addresses all the comments above, except that
> Volodya> request for 'closure' pointer to read_command_lines_1 if
> Volodya> addressed by a comment saying why we don't do that.
> 
> There still isn't a header comment on read_next_line.  Just something
> like "helper function for use with recurse_read_control_structure" would
> be fine.  This is ok with the addition of a comment.

Thanks. Checked in with that change, see attached.

- Volodya

[-- Attachment #2: a.diff --]
[-- Type: text/x-patch, Size: 7740 bytes --]

commit f4dc5936f411766cf8679d752e416e8644caafc8
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..6e90bf2 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,15 @@ 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.  */
+/* Read next line from stdout.  Passed to read_command_line_1 and
+   recurse_read_control_structure whenever we need to read commands
+   from stdout.  */
 
-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 +865,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 +986,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 +1017,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 +1084,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 +1111,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 +1127,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 +1178,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 +1209,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 **);
 

[-- Attachment #3: b.diff --]
[-- Type: text/x-patch, Size: 11343 bytes --]

commit dad2bfb291f5848c6da4260bf8adbc1086128ccf
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;
+}
 
 \f
 /* 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

Re: [RFA] Implement -break-commands

[Vladimir Prus]

On Thursday 30 July 2009 Eli Zaretskii wrote:

> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Thu, 30 Jul 2009 10:23:45 +0400
> > Cc: gdb-patches@sources.redhat.com
> > 
> > Eli, I've also applied your documentation suggestions. Is this OK?
> 
> Yes, thanks.

I've just realized I failed to provide a patch for NEWS. What about the
below?

Thanks,
Volodya

--- gdb/NEWS    31 Jul 2009 15:35:37 -0000      1.324
+++ gdb/NEWS    3 Aug 2009 12:41:43 -0000
@@ -257,6 +257,9 @@ detach inferior NUM
 kill inferior NUM
   Kill inferior number NUM.

+-break-commands
+  Set breakpoint commands using the GDB/MI interface
+
 * New options

 set spu stop-on-load

Re: [RFA] Implement -break-commands

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Mon, 3 Aug 2009 16:42:31 +0400
> Cc: tromey@redhat.com,
>  gdb-patches@sources.redhat.com
> 
> I've just realized I failed to provide a patch for NEWS. What about the
> below?

Yes, okay.  But if other MI-related features are committed before the
release, please make a new section for new MI features, and put this
and the additional news there, so that all MI-related news are kept
together.

Thanks.