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

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Fri, 13 Jan 2006 16:48:37 +0200
> From: Eli Zaretskii <eliz@gnu.org>

Yes, it really has been a year since I submitted this.  Only now I
found time to run the test suite and make sure these changes don't
break anything.  To remind everyone what is this all about, most of my
original message is reproduced below.

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

Here's the renewed patch.  Okay to commit?

(I will post a first attempt to add a test for this in a moment.)

2007-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.~1~	2007-01-09 16:34:29.000000000 -0500
+++ defs.h	2007-01-13 06:36:43.752682122 -0500
@@ -691,6 +691,7 @@
     continue_control,
     while_control,
     if_control,
+    commands_control,
     invalid_control
   };
 
--- breakpoint.h.~1~	2007-01-09 12:58:50.000000000 -0500
+++ breakpoint.h	2007-01-13 06:36:43.752682122 -0500
@@ -759,6 +759,10 @@
 
 extern void enable_watchpoints_after_interactive_call_stop (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);
 
 extern void clear_breakpoint_hit_counts (void);
 
--- breakpoint.c.~1~	2007-01-09 12:58:49.000000000 -0500
+++ breakpoint.c	2007-01-13 06:36:43.762682122 -0500
@@ -403,6 +403,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.  */ 
@@ -647,6 +649,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.~1~	2007-01-09 12:59:00.000000000 -0500
+++ cli/cli-script.c	2007-01-13 09:20:48.252682122 -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"
@@ -82,7 +83,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));
@@ -115,7 +116,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;
     }
@@ -207,6 +208,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) */
@@ -292,7 +310,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;
@@ -498,9 +516,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;
     }
 
@@ -849,6 +878,14 @@ read_next_line (struct command_line **co
         first_arg++;
       *command = build_command_line (if_control, first_arg);
     }
+  else 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 if (p1 - p == 10 && !strncmp (p, "loop_break", 10))
     {
       *command = (struct command_line *)
@@ -924,9 +961,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;
 	    }
@@ -974,7 +1012,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);
@@ -1045,7 +1084,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);