[RFA] Allow use of breakpoint commands inside `if' or `while'

[Eli Zaretskii <eliz at gnu dot org>]

Consider the following snippet from a .gdbinit file:

    set $tem = 1
    if $tem == 2
      break init_fns
    else
      break abort
      tbreak init_sys_modes
      commands
	silent
	set $tem = 3
	continue
      end
    end

(The names of functions are from Emacs; if you want to try this with a
different program, replace them with arbitrary functions from that
program.)

Try this file as shown, and then again with the second line modified
to say "if $tem == 1".  The first case causes GDB to complain about
"silent":

    .gdbinit:12: Error in sourced command file:
    Undefined command: "silent".  Try "help".

The second case produces a complaint about "end":

    .gdbinit:12: Error in sourced command file:
    This command cannot be used at the top level.

The problem here is that cli-script.c doesn't treat the `commands'
command specially inside a block of canned commands (i.e. inside a
body of an `if' or `while' command).  So in the first case, it
actually _invokes_ "commands" when the `else' branch is run, and
commands_command tries to read the commands from input, but finds
nothing because everything was already read (when `if' was parsed).
So it defines an empty command list (which is already a bug), and then
GDB sees `silent' and complains.  In the second case, the extra "end",
which was not handled as ending the commands block, causes GDB to
think that it is used at top level.

AFAICS, this problem was in GDB since when the control flow commands
were introduced.

The patches below fix this problem.

Okay to commit?

2006-01-13  Eli Zaretskii  <eliz@gnu.org>

	* cli/cli-script.c: Include breakpoint.h.
	(build_command_line): Require arguments only for `if' and `while'
	commands.
	(get_command_line, execute_user_command, execute_control_command):
	Fix wording of warning messages.
	(print_command_lines): Print breakpoint commands.
	(execute_control_command): Call commands_from_control_command to
	handle the `commands' command inside a body of a flow-control command.
	(read_next_line): Recognize the `commands' command and build a
	command line structure for it.
	(recurse_read_control_structure, read_command_lines): Handle
	`commands' similarly to `if' and `while'.

	* breakpoint.c (get_number_trailer): Document the special meaning
	of NULL as the first argument PP.
	(commands_from_control_command): New function.

	* breakpoint.h (commands_from_control_command): Add prototype.

	* defs.h (commands_control): New enumerated value for enum
	command_control_type.


--- defs.h~0	2005-12-17 17:33:59.000000000 -0500
+++ defs.h	2006-01-13 06:37:07.422360288 -0500
@@ -664,6 +664,7 @@
     continue_control,
     while_control,
     if_control,
+    commands_control,
     invalid_control
   };
 

--- breakpoint.h~0	2005-12-17 17:33:59.000000000 -0500
+++ breakpoint.h	2006-01-13 07:28:39.660268672 -0500
@@ -796,6 +796,10 @@
    remove fails. */
 extern int remove_hw_watchpoints (void);
 
+/* For script interpreters that need to define breakpoint commands
+   after they've already read the commands into a struct command_line.  */
+extern enum command_control_type commands_from_control_command
+  (char *arg, struct command_line *cmd);
 
 /* Indicator of whether exception catchpoints should be nuked between
    runs of a program.  */

--- breakpoint.c~0	2005-12-17 17:33:59.000000000 -0500
+++ breakpoint.c	2006-01-13 08:31:12.704719192 -0500
@@ -385,6 +385,8 @@ int default_breakpoint_line;
    Currently the string can either be a number or "$" followed by the name
    of a convenience variable.  Making it an expression wouldn't work well
    for map_breakpoint_numbers (e.g. "4 + 5 + 6").
+
+   If the string is a NULL pointer, that denotes the last breakpoint.
    
    TRAILER is a character which can be found after the number; most
    commonly this is `-'.  If you don't want a trailer, use \0.  */ 
@@ -629,6 +631,52 @@ commands_command (char *arg, int from_tt
     }
   error (_("No breakpoint number %d."), bnum);
 }
+
+/* Like commands_command, but instead of reading the commands from
+   input stream, takes them from an already parsed command structure.
+
+   This is used by cli-script.c to DTRT with breakpoint commands
+   that are part of if and while bodies.  */
+enum command_control_type
+commands_from_control_command (char *arg, struct command_line *cmd)
+{
+  struct breakpoint *b;
+  char *p;
+  int bnum;
+
+  /* If we allowed this, we would have problems with when to
+     free the storage, if we change the commands currently
+     being read from.  */
+
+  if (executing_breakpoint_commands)
+    error (_("Can't use the \"commands\" command among a breakpoint's commands."));
+
+  /* An empty string for the breakpoint number means the last
+     breakpoint, but get_number expects a NULL pointer.  */
+  if (arg && !*arg)
+    p = NULL;
+  else
+    p = arg;
+  bnum = get_number (&p);
+
+  if (p && *p)
+    error (_("Unexpected extra arguments following breakpoint number."));
+
+  ALL_BREAKPOINTS (b)
+    if (b->number == bnum)
+      {
+	free_command_lines (&b->commands);
+	if (cmd->body_count != 1)
+	  error (_("Invalid \"commands\" block structure."));
+	/* We need to copy the commands because if/while will free the
+	   list after it finishes execution.  */
+	b->commands = copy_command_lines (cmd->body_list[0]);
+	breakpoints_changed ();
+	breakpoint_modify_event (b->number);
+	return simple_control;
+    }
+  error (_("No breakpoint number %d."), bnum);
+}
 
 /* Like target_read_memory() but if breakpoints are inserted, return
    the shadow contents instead of the breakpoints themselves.

--- cli/cli-script.c~0	2005-12-17 17:40:17.000000000 -0500
+++ cli/cli-script.c	2006-01-13 07:52:46.751277400 -0500
@@ -30,6 +30,7 @@
 #include "gdb_string.h"
 #include "exceptions.h"
 #include "top.h"
+#include "breakpoint.h"
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "cli/cli-script.h"
@@ -73,7 +74,7 @@ build_command_line (enum command_control
 {
   struct command_line *cmd;
 
-  if (args == NULL)
+  if (args == NULL && (type == if_control || type == while_control))
     error (_("if/while commands require arguments."));
 
   cmd = (struct command_line *) xmalloc (sizeof (struct command_line));
@@ -106,7 +107,7 @@ get_command_line (enum command_control_t
   /* Read in the body of this command.  */
   if (recurse_read_control_structure (cmd) == invalid_control)
     {
-      warning (_("Error reading in control structure."));
+      warning (_("Error reading in canned sequence of commands."));
       do_cleanups (old_chain);
       return NULL;
     }
@@ -198,6 +199,23 @@ print_command_lines (struct ui_out *uiou
 	  continue;
 	}
 
+      /* A commands command.  Print the breakpoint commands and continue.  */
+      if (list->control_type == commands_control)
+	{
+	  if (*(list->line))
+	    ui_out_field_fmt (uiout, NULL, "commands %s", list->line);
+	  else
+	    ui_out_field_string (uiout, NULL, "commands");
+	  ui_out_text (uiout, "\n");
+	  print_command_lines (uiout, *list->body_list, depth + 1);
+	  if (depth)
+	    ui_out_spaces (uiout, 2 * depth);
+	  ui_out_field_string (uiout, NULL, "end");
+	  ui_out_text (uiout, "\n");
+	  list = list->next;
+	  continue;
+	}
+
       /* ignore illegal command type and try next */
       list = list->next;
     }				/* while (list) */
@@ -277,7 +292,7 @@ execute_user_command (struct cmd_list_el
       ret = execute_control_command (cmdlines);
       if (ret != simple_control && ret != break_control)
 	{
-	  warning (_("Error in control structure."));
+	  warning (_("Error executing canned sequence of commands."));
 	  break;
 	}
       cmdlines = cmdlines->next;
@@ -423,9 +438,20 @@ execute_control_command (struct command_
 
 	break;
       }
+    case commands_control:
+      {
+	/* Breakpoint commands list, record the commands in the breakpoint's
+	   command list and return.  */
+	new_line = insert_args (cmd->line);
+	if (!new_line)
+	  break;
+	make_cleanup (free_current_contents, &new_line);
+	ret = commands_from_control_command (new_line, cmd);
+	break;
+      }
 
     default:
-      warning (_("Invalid control type in command structure."));
+      warning (_("Invalid control type in canned commands structure."));
       break;
     }
 
@@ -783,6 +809,14 @@ read_next_line (struct command_line **co
       (*command)->body_count = 0;
       (*command)->body_list = NULL;
     }
+  if (p1 - p >= 8 && !strncmp (p, "commands", 8))
+    {
+      char *first_arg;
+      first_arg = p + 8;
+      while (first_arg < p1 && isspace (*first_arg))
+        first_arg++;
+      *command = build_command_line (commands_control, first_arg);
+    }
   else
     {
       /* A normal command.  */
@@ -838,9 +872,10 @@ recurse_read_control_structure (struct c
       if (val == end_command)
 	{
 	  if (current_cmd->control_type == while_control
-	      || current_cmd->control_type == if_control)
+	      || current_cmd->control_type == if_control
+	      || current_cmd->control_type == commands_control)
 	    {
-	      /* Success reading an entire control structure.  */
+	      /* Success reading an entire canned sequence of commands.  */
 	      ret = simple_control;
 	      break;
 	    }
@@ -888,7 +923,8 @@ recurse_read_control_structure (struct c
       /* If the latest line is another control structure, then recurse
          on it.  */
       if (next->control_type == while_control
-	  || next->control_type == if_control)
+	  || next->control_type == if_control
+	  || next->control_type == commands_control)
 	{
 	  control_level++;
 	  ret = recurse_read_control_structure (next);
@@ -955,7 +991,8 @@ read_command_lines (char *prompt_arg, in
 	}
 
       if (next->control_type == while_control
-	  || next->control_type == if_control)
+	  || next->control_type == if_control
+	  || next->control_type == commands_control)
 	{
 	  control_level++;
 	  ret = recurse_read_control_structure (next);