Unify actions and commands

* defs.h (read_command_lines, read_command_lines_1): New
	parameters validator and closure.
	* tracepoint.h (struct action_line): Remove.
	* breakpoint.h (struct breakpoint): Remove the 'actions'
	field.
	* defs.h (enum command_control_type): New value
	while_stepping_control.
	(struct command_line): Add comments.
	* breakpoint.c (breakoint_is_tracepoint): New.
	(breakpoint_set_commands): For tracepoints,
	verify the commands are permissible.
	(check_tracepoint_commands): New.
	(commands_command): Require that each new line is validated using
	check_tracepoint_command, if we set commands for a tracepoint.
	(create_tracepoint_from_upload): Likewise.
	(print_one_breakpoint_location): Remove the code to print
	actions specifically.
	(tracepoint_save_command): Relay to print_command_lines.
	* cli/cli-script.c (process_next_line): New parameters validator
	and closure. Handle 'while-stepping'. Call validator if not null.
	(read_command_lines, read_command_lines1): Likewise.
	(recurse_read_control_structure): New parameters validator and
	closure. Handle while_stepping_control.
	(print_command_lines): Handle while-stepping.
	(get_command_line, define_command, document_command): Adjust.
	* remote.c (remote_download_tracepoint): Adjust.
	* tracepoint.c (make_cleanup_free_actions, read_actions)
	(free_actions, do_free_actions_cleanup): Remove.
	(trace_actions_command): Use read_command_lines.
	(validate_actionline): Use error in one place.
	(encode_actions_1): New, extracted from...
	(encode_actions): ...this. Also use cleanups for exception
	safety.
	(trace_dump_command): Adjust.
	* mi/mi-cmd-break (mi_cmd_break_commands): Validate commands if
	it's tracepoint.

gdb/ChangeLog

@@ -1,3 +1,45 @@
+2010-03-24  Vladimir Prus  <vladimir@codesourcery.com>
+
+	Unify actions and commands
+
+	* defs.h (read_command_lines, read_command_lines_1): New
+	parameters validator and closure.
+	* tracepoint.h (struct action_line): Remove.
+	* breakpoint.h (struct breakpoint): Remove the 'actions'
+	field.
+	* defs.h (enum command_control_type): New value
+	while_stepping_control.
+	(struct command_line): Add comments.
+	* breakpoint.c (breakoint_is_tracepoint): New.
+	(breakpoint_set_commands): For tracepoints,
+	verify the commands are permissible.
+	(check_tracepoint_commands): New.
+	(commands_command): Require that each new line is validated using
+	check_tracepoint_command, if we set commands for a tracepoint.
+	(create_tracepoint_from_upload): Likewise.
+	(print_one_breakpoint_location): Remove the code to print
+	actions specifically.
+	(tracepoint_save_command): Relay to print_command_lines.
+	* cli/cli-script.c (process_next_line): New parameters validator
+	and closure. Handle 'while-stepping'. Call validator if not null.
+	(read_command_lines, read_command_lines1): Likewise.
+	(recurse_read_control_structure): New parameters validator and
+	closure. Handle while_stepping_control.
+	(print_command_lines): Handle while-stepping.
+	(get_command_line, define_command, document_command): Adjust.
+	* remote.c (remote_download_tracepoint): Adjust.
+	* tracepoint.c (make_cleanup_free_actions, read_actions)
+	(free_actions, do_free_actions_cleanup): Remove.
+	(trace_actions_command): Use read_command_lines.
+	(validate_actionline): Use error in one place.
+	(encode_actions_1): New, extracted from...
+	(encode_actions): ...this. Also use cleanups for exception
+	safety.
+	(trace_dump_command): Adjust.
+	* mi/mi-cmd-break (mi_cmd_break_commands): Validate commands if
+	it's tracepoint.
+
+=======
 2010-03-23  Mike Frysinger  <vapier@gentoo.org>
 
 	* Makefile.in (OPCODES_CFLAGS): Add -I$(OPCODES_SRC)/..

gdb/NEWS

@@ -20,6 +20,11 @@
 
 ** New methods gdb.target_charset and gdb.target_wide_charset.
 
+* Tracepoint actions were unified with breakpoint commands. In particular,
+there are no longer differences in "info break" output for breakpoints and
+tracepoints and the "commands" command can be used for both tracepoints and
+regular breakpoints.
+
 * New targets
 
 ARM Symbian			arm*-*-symbianelf*

gdb/breakpoint.c

@@ -692,17 +692,109 @@ condition_command (char *arg, int from_tty)
   error (_("No breakpoint number %d."), bnum);
 }
 
-/* Set the command list of B to COMMANDS.  */
+/* Check that COMMAND do not contain commands that are suitable
+   only for tracepoints and not suitable for ordinary breakpoints.
+   Throw if any such commands is found.
+*/
+static void
+check_no_tracepoint_commands (struct command_line *commands)
+{
+  struct command_line *c;
+  for (c = commands; c; c = c->next)
+    {
+      int i;
+
+      if (c->control_type == while_stepping_control)
+	error (_("The 'while-stepping' command can only be used for tracepoints"));
+
+      for (i = 0; i < c->body_count; ++i)
+	check_no_tracepoint_commands ((c->body_list)[i]);
+
+      /* Not that command parsing removes leading whitespace and comment
+	 lines and also empty lines. So, we only need to check for
+	 command directly.  */
+      if (strstr (c->line, "collect ") == c->line)
+	error (_("The 'collect' command can only be used for tracepoints"));
+
+      if (strstr (c->line, "eval ") == c->line)
+	error (_("The 'eval' command can only be used for tracepoints"));
+    }
+}
+
+int
+breakpoint_is_tracepoint (const struct breakpoint *b)
+{
+  switch (b->type)
+    {
+    case bp_tracepoint:
+    case bp_fast_tracepoint:
+      return 1;
+    default:
+      return 0;
+
+    }
+}
+
+/* Set the command list of B to COMMANDS.  If breakpoint is tracepoint,
+   validate that only allowed commands are included.
+*/
 
 void
 breakpoint_set_commands (struct breakpoint *b, struct command_line *commands)
 {
+  if (breakpoint_is_tracepoint (b))
+    {
+      /* We need to verify that each top-level element of commands
+	 is valid for tracepoints, that there's at most one while-stepping
+	 element, and that while-stepping's body has valid tracing commands
+	 excluding nested while-stepping.  */
+      struct command_line *c;
+      struct command_line *while_stepping = 0;
+      for (c = commands; c; c = c->next)
+	{
+	  char *l = c->line;
+	  if (c->control_type == while_stepping_control)
+	    {
+	      if (b->type == bp_fast_tracepoint)
+		error (_("The 'while-stepping' command cannot be used for fast tracepoint"));
+
+	      if (while_stepping)
+		error (_("The 'while-stepping' command can be used only once"));
+	      else
+		while_stepping = c;
+	    }
+	}
+      if (while_stepping)
+	{
+	  struct command_line *c2;
+
+	  gdb_assert (while_stepping->body_count == 1);
+	  c2 = while_stepping->body_list[0];
+	  for (; c2; c2 = c2->next)
+	    {
+	      char *l = c2->line;
+	      if (c2->control_type == while_stepping_control)
+		error (_("The 'while-stepping' command cannot be nested"));
+	    }
+	}
+    }
+  else
+    {
+      check_no_tracepoint_commands (commands);
+    }
+
   free_command_lines (&b->commands);
   b->commands = commands;
   breakpoints_changed ();
   observer_notify_breakpoint_modified (b->number);
 }
 
+void check_tracepoint_command (char *line, void *closure)
+{
+  struct breakpoint *b = closure;
+  validate_actionline (&line, b);
+}
+
 static void
 commands_command (char *arg, int from_tty)
 {
@@ -730,7 +822,12 @@ commands_command (char *arg, int from_tty)
 	char *tmpbuf = xstrprintf ("Type commands for when breakpoint %d is hit, one per line.", 
 				 bnum);
 	struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
-	l = read_command_lines (tmpbuf, from_tty, 1);
+
+	if (breakpoint_is_tracepoint (b))
+	  l = read_command_lines (tmpbuf, from_tty, 1,
+				  check_tracepoint_command, b);
+	else
+	  l = read_command_lines (tmpbuf, from_tty, 1, 0, 0);
 	do_cleanups (cleanups);
 	breakpoint_set_commands (b, l);
 	return;
@@ -4539,26 +4636,6 @@ print_one_breakpoint_location (struct breakpoint *b,
       ui_out_text (uiout, " \n");
     }
 
-  if (!part_of_multiple && b->step_count)
-    {
-      annotate_field (11);
-      ui_out_text (uiout, "\tstep count ");
-      ui_out_field_int (uiout, "step", b->step_count);
-      ui_out_text (uiout, " \n");
-    }
-
-  if (!part_of_multiple && b->actions)
-    {
-      struct action_line *action;
-      annotate_field (12);
-      for (action = b->actions; action; action = action->next)
-	{
-	  ui_out_text (uiout, "      A\t");
-	  ui_out_text (uiout, action->action);
-	  ui_out_text (uiout, "\n");
-	}
-    }
-
   if (ui_out_is_mi_like_p (uiout) && !part_of_multiple)
     {
       if (b->addr_string)
@@ -10318,12 +10395,11 @@ tracepoint_save_command (char *args, int from_tty)
 {
   struct breakpoint *tp;
   int any_tp = 0;
-  struct action_line *line;
+  struct command_line *line;
-  FILE *fp;
+  char *pathname;
-  char *i1 = "    ", *i2 = "      ";
-  char *indent, *actionline, *pathname;
   char tmp[40];
   struct cleanup *cleanup;
+  struct ui_file *fp;
 
   if (args == 0 || *args == 0)
     error (_("Argument required (file name in which to save tracepoints)"));
@@ -10342,50 +10418,42 @@ tracepoint_save_command (char *args, int from_tty)
 
   pathname = tilde_expand (args);
   cleanup = make_cleanup (xfree, pathname);
-  fp = fopen (pathname, "w");
+  fp = gdb_fopen (pathname, "w");
   if (!fp)
     error (_("Unable to open file '%s' for saving tracepoints (%s)"),
 	   args, safe_strerror (errno));
-  make_cleanup_fclose (fp);
+  make_cleanup_ui_file_delete (fp);
   
   ALL_TRACEPOINTS (tp)
   {
     if (tp->addr_string)
-      fprintf (fp, "trace %s\n", tp->addr_string);
+      fprintf_unfiltered (fp, "trace %s\n", tp->addr_string);
     else
       {
 	sprintf_vma (tmp, tp->loc->address);
-	fprintf (fp, "trace *0x%s\n", tmp);
+	fprintf_unfiltered (fp, "trace *0x%s\n", tmp);
       }
 
     if (tp->pass_count)
-      fprintf (fp, "  passcount %d\n", tp->pass_count);
+      fprintf_unfiltered (fp, "  passcount %d\n", tp->pass_count);
 
-    if (tp->actions)
+    if (tp->commands)
       {
-	fprintf (fp, "  actions\n");
+	volatile struct gdb_exception ex;	
-	indent = i1;
-	for (line = tp->actions; line; line = line->next)
-	  {
-	    struct cmd_list_element *cmd;
 
-	    QUIT;		/* allow user to bail out with ^C */
+	fprintf_unfiltered (fp, "  actions\n");
-	    actionline = line->action;
-	    while (isspace ((int) *actionline))
-	      actionline++;
 	
-	    fprintf (fp, "%s%s\n", indent, actionline);
+	ui_out_redirect (uiout, fp);
-	    if (*actionline != '#')	/* skip for comment lines */
+	TRY_CATCH (ex, RETURN_MASK_ERROR)
 	  {
-		cmd = lookup_cmd (&actionline, cmdlist, "", -1, 1);
+	    print_command_lines (uiout, tp->commands, 2);
-		if (cmd == 0)
-		  error (_("Bad action list item: %s"), actionline);
-		if (cmd_cfunc_eq (cmd, while_stepping_pseudocommand))
-		  indent = i2;
-		else if (cmd_cfunc_eq (cmd, end_actions_pseudocommand))
-		  indent = i1;
-	      }
 	  }
+	ui_out_redirect (uiout, NULL);
+
+	if (ex.reason < 0)
+	  throw_exception (ex);
+
+	fprintf_unfiltered (fp, "  end\n");
       }
   }
   do_cleanups (cleanup);

gdb/breakpoint.h

@@ -518,9 +518,6 @@ struct breakpoint
        disabling/ending.  */
     int pass_count;
 
-    /* Chain of action lines to execute when this tracepoint is hit.  */
-    struct action_line *actions;
-
     /* The number of the tracepoint on the target.  */
     int number_on_target;
   };
@@ -1008,4 +1005,10 @@ extern struct breakpoint *get_tracepoint_by_number (char **arg, int multi_p,
    is newly allocated; the caller should free when done with it.  */
 extern VEC(breakpoint_p) *all_tracepoints (void);
 
+extern int breakpoint_is_tracepoint (const struct breakpoint *b);
+
+/* Function that can be passed to read_command_line to validate
+   that each command is suitable for tracepoint command list.  */
+extern void check_tracepoint_command (char *line, void *closure);
+
 #endif /* !defined (BREAKPOINT_H) */

gdb/cli/cli-script.c

@@ -40,7 +40,9 @@
 
 static enum command_control_type
 recurse_read_control_structure (char * (*read_next_line_func) (void),
-				struct command_line *current_cmd);
+				struct command_line *current_cmd,
+				void (*validator)(char *, void *),
+				void *closure);
 
 static char *insert_args (char *line);
 
@@ -117,7 +119,8 @@ 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 (read_next_line, cmd) == invalid_control)
+  if (recurse_read_control_structure (read_next_line, cmd, 0, 0)
+      == invalid_control)
     {
       warning (_("Error reading in canned sequence of commands."));
       do_cleanups (old_chain);
@@ -172,9 +175,16 @@ print_command_lines (struct ui_out *uiout, struct command_line *cmd,
 	}
 
       /* A while command.  Recursively print its subcommands and continue.  */
-      if (list->control_type == while_control)
+      if (list->control_type == while_control
+	  || list->control_type == while_stepping_control)
 	{
+	  /* For while-stepping, the line includes the 'while-stepping' token.
+	     See comment in process_next_line for explanation.  Here,
+	     take care not print 'while-stepping' twice.  */
+	  if (list->control_type == while_control)
 	    ui_out_field_fmt (uiout, NULL, "while %s", list->line);
+	  else
+	    ui_out_field_string (uiout, NULL, list->line);
 	  ui_out_text (uiout, "\n");
 	  print_command_lines (uiout, *list->body_list, depth + 1);
 	  if (depth)
@@ -876,7 +886,8 @@ read_next_line (void)
    Otherwise, only "end" is recognized.  */
 
 static enum misc_command_type
-process_next_line (char *p, struct command_line **command, int parse_commands)
+process_next_line (char *p, struct command_line **command, int parse_commands,
+		   void (*validator)(char *, void *), void *closure)
 {
   char *p_end;
   char *p_start;
@@ -920,7 +931,22 @@ process_next_line (char *p, struct command_line **command, int parse_commands)
 
       /* Check for while, if, break, continue, etc and build a new command
 	 line structure for them.  */
-      if (p_end - p > 5 && !strncmp (p, "while", 5))
+      if ((p_end - p >= 14 && !strncmp (p, "while-stepping", 14))
+	  || (p_end -p >= 2 && !strncmp (p, "ws", 2)))
+	{
+	  /* Because validate_actionline and encode_action lookup
+	     command's line as command, we need the line to
+	     include 'while-stepping'.
+
+	     For 'ws' alias, the command will have 'ws', not expanded
+	     to 'while-stepping'. This is intentional -- we don't
+	     really want frontend to send a command list with 'ws',
+	     and next break-info returning command line with 'while-stepping'.
+	     This should work, but might cause the breakpoint to be marked as
+	     changed while it's actually not.  */
+	  *command = build_command_line (while_stepping_control, p);
+	}
+      else if (p_end - p > 5 && !strncmp (p, "while", 5))
 	{
 	  char *first_arg;
 	  first_arg = p + 5;
@@ -986,6 +1012,20 @@ process_next_line (char *p, struct command_line **command, int parse_commands)
       (*command)->body_list = NULL;
     }
 
+  if (validator)
+    {
+      volatile struct gdb_exception ex;
+      TRY_CATCH (ex, RETURN_MASK_ALL)
+	{
+	  validator ((*command)->line, closure);
+	}
+      if (ex.reason < 0)
+	{
+	  xfree (*command);
+	  throw_exception (ex);
+	}
+    }
+
   /* Nothing special.  */
   return ok_command;
 }
@@ -998,7 +1038,9 @@ process_next_line (char *p, struct command_line **command, int parse_commands)
 
 static enum command_control_type
 recurse_read_control_structure (char * (*read_next_line_func) (void),
-				struct command_line *current_cmd)
+				struct command_line *current_cmd,
+				void (*validator)(char *, void *),
+				void *closure)
 {
   int current_body, i;
   enum misc_command_type val;
@@ -1023,7 +1065,8 @@ recurse_read_control_structure (char * (*read_next_line_func) (void),
 
       next = NULL;
       val = process_next_line (read_next_line_func (), &next, 
-			       current_cmd->control_type != python_control);
+			       current_cmd->control_type != python_control,
+			       validator, closure);
 
       /* Just skip blanks and comments.  */
       if (val == nop_command)
@@ -1032,6 +1075,7 @@ recurse_read_control_structure (char * (*read_next_line_func) (void),
       if (val == end_command)
 	{
 	  if (current_cmd->control_type == while_control
+	      || current_cmd->control_type == while_stepping_control
 	      || current_cmd->control_type == if_control
 	      || current_cmd->control_type == python_control
 	      || current_cmd->control_type == commands_control)
@@ -1084,12 +1128,14 @@ recurse_read_control_structure (char * (*read_next_line_func) (void),
       /* If the latest line is another control structure, then recurse
          on it.  */
       if (next->control_type == while_control
+	  || next->control_type == while_stepping_control
 	  || next->control_type == if_control
 	  || next->control_type == python_control
 	  || next->control_type == commands_control)
 	{
 	  control_level++;
-	  ret = recurse_read_control_structure (read_next_line_func, next);
+	  ret = recurse_read_control_structure (read_next_line_func, next,
+						validator, closure);
 	  control_level--;
 
 	  if (ret != simple_control)
@@ -1114,7 +1160,8 @@ recurse_read_control_structure (char * (*read_next_line_func) (void),
 #define END_MESSAGE "End with a line saying just \"end\"."
 
 struct command_line *
-read_command_lines (char *prompt_arg, int from_tty, int parse_commands)
+read_command_lines (char *prompt_arg, int from_tty, int parse_commands,
+		    void (*validator)(char *, void *), void *closure)
 {
   struct command_line *head;
 
@@ -1132,7 +1179,8 @@ read_command_lines (char *prompt_arg, int from_tty, int parse_commands)
 	}
     }
 
-  head = read_command_lines_1 (read_next_line, parse_commands);
+  head = read_command_lines_1 (read_next_line, parse_commands,
+			       validator, closure);
 
   if (deprecated_readline_end_hook && from_tty && input_from_terminal_p ())
     {
@@ -1145,7 +1193,8 @@ read_command_lines (char *prompt_arg, int from_tty, int parse_commands)
    obtained using READ_NEXT_LINE_FUNC.  */
 
 struct command_line *
-read_command_lines_1 (char * (*read_next_line_func) (void), int parse_commands)
+read_command_lines_1 (char * (*read_next_line_func) (void), int parse_commands,
+		      void (*validator)(char *, void *), void *closure)
 {
   struct command_line *head, *tail, *next;
   struct cleanup *old_chain;
@@ -1159,7 +1208,8 @@ read_command_lines_1 (char * (*read_next_line_func) (void), int parse_commands)
   while (1)
     {
       dont_repeat ();
-      val = process_next_line (read_next_line_func (), &next, parse_commands);
+      val = process_next_line (read_next_line_func (), &next, parse_commands,
+			       validator, closure);
 
       /* Ignore blank lines or comments.  */
       if (val == nop_command)
@@ -1180,10 +1230,12 @@ read_command_lines_1 (char * (*read_next_line_func) (void), int parse_commands)
       if (next->control_type == while_control
 	  || next->control_type == if_control
 	  || next->control_type == python_control
-	  || next->control_type == commands_control)
+	  || next->control_type == commands_control
+	  || next->control_type == while_stepping_control)
 	{
 	  control_level++;
-	  ret = recurse_read_control_structure (read_next_line_func, next);
+	  ret = recurse_read_control_structure (read_next_line_func, next,
+						validator, closure);
 	  control_level--;
 
 	  if (ret == invalid_control)
@@ -1426,7 +1478,7 @@ define_command (char *comname, int from_tty)
       *tem = tolower (*tem);
 
   sprintf (tmpbuf, "Type commands for definition of \"%s\".", comfull);
-  cmds = read_command_lines (tmpbuf, from_tty, 1);
+  cmds = read_command_lines (tmpbuf, from_tty, 1, 0, 0);
 
   if (c && c->class == class_user)
     free_command_lines (&c->user_commands);
@@ -1475,7 +1527,7 @@ document_command (char *comname, int from_tty)
     error (_("Command \"%s\" is built-in."), comfull);
 
   sprintf (tmpbuf, "Type documentation for \"%s\".", comfull);
-  doclines = read_command_lines (tmpbuf, from_tty, 0);
+  doclines = read_command_lines (tmpbuf, from_tty, 0, 0, 0);
 
   if (c->doc)
     xfree (c->doc);

gdb/defs.h

@@ -691,6 +691,7 @@ enum command_control_type
     if_control,
     commands_control,
     python_control,
+    while_stepping_control,
     invalid_control
   };
 
@@ -702,12 +703,21 @@ struct command_line
     struct command_line *next;
     char *line;
     enum command_control_type control_type;
+    /* The number of elements in body_list.  */
     int body_count;
+    /* For composite commands, the nested lists of
+       commands. For example, for "if" command this
+       will contain the then branch and the else
+       branch, if that is available.  */
     struct command_line **body_list;
   };
 
-extern struct command_line *read_command_lines (char *, int, int);
+extern struct command_line *read_command_lines (char *, int, int,
-extern struct command_line *read_command_lines_1 (char * (*) (void), int);
+						void (*)(char *, void *),
+						void *);
+extern struct command_line *read_command_lines_1 (char * (*) (void), int,
+						  void (*)(char *, void *),
+						  void *);
 
 extern void free_command_lines (struct command_line **);
 

gdb/mi/mi-cmd-break.c

@@ -271,7 +271,12 @@ mi_cmd_break_commands (char *command, char **argv, int argc)
   mi_command_line_array_ptr = 1;
   mi_command_line_array_cnt = argc;
 
-  break_command = read_command_lines_1 (mi_read_next_line, 1);
+  if (breakpoint_is_tracepoint (b))
+    break_command = read_command_lines_1 (mi_read_next_line, 1,
+					  check_tracepoint_command, b);
+  else
+    break_command = read_command_lines_1 (mi_read_next_line, 1, 0, 0);
+
   breakpoint_set_commands (b, break_command);
 }
 

gdb/remote.c

@@ -9337,14 +9337,14 @@ remote_download_tracepoint (struct breakpoint *t)
 	    warning (_("Target does not support conditional tracepoints, ignoring tp %d cond"), t->number);
 	}
 
-      if (t->actions || *default_collect)
+  if (t->commands || *default_collect)
 	strcat (buf, "-");
       putpkt (buf);
       remote_get_noisy_reply (&target_buf, &target_buf_size);
       if (strcmp (target_buf, "OK"))
 	error (_("Target does not support tracepoints."));
 
-      if (!t->actions && !*default_collect)
+  if (!t->commands && !*default_collect)
 	continue;
 
       /* do_single_steps (t); */

gdb/testsuite/ChangeLog

@@ -1,3 +1,10 @@
+2010-03-24  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* lib/trace-support.exp (gdb_trace_setactions):
+	Don't expect whitespace after ">" prompt.
+	* gdb.trace/actions.exp: Adjust for output changes.
+	* gdb.trace/while-stepping.exp: Likewise.
+
 2010-03-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* dg-extract-results.sh: Sync with GCC HEAD (import r155655, r157175

gdb/testsuite/gdb.trace/actions.exp

@@ -88,8 +88,7 @@ gdb_trace_setactions "5.1b: set actions for first tracepoint" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_char_test.
+\[\t \]+collect gdb_char_test.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
 		"5.1c: verify actions set for first tracepoint"
@@ -101,11 +100,9 @@ gdb_trace_setactions "5.1d: set actions for second tracepoint" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_char_test.
+\[\t \]+collect gdb_char_test.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_short_test.
+\[\t \]+collect gdb_short_test.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
 		"5.1e: verify actions set for second tracepoint"
 
@@ -116,14 +113,11 @@ gdb_trace_setactions "5.2a: set actions for last (default) tracepoint" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_char_test.
+\[\t \]+collect gdb_char_test.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_short_test.
+\[\t \]+collect gdb_short_test.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_long_test.
+\[\t \]+collect gdb_long_test." \
-\[\t \]+A\[\t \]+end." \
 		"5.1e: verify actions set for second tracepoint"
 
 # 5.3 replace actions set earlier
@@ -135,14 +129,11 @@ gdb_trace_setactions "5.3a: reset actions for first tracepoint" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_struct1_test.
+\[\t \]+collect gdb_struct1_test.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_short_test.
+\[\t \]+collect gdb_short_test.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_long_test.
+\[\t \]+collect gdb_long_test." \
-\[\t \]+A\[\t \]+end." \
 		"5.3b: verify actions set for first tracepoint"
 
 #
@@ -184,7 +175,7 @@ gdb_test "actions [expr $trcpt2 + $trcpt3]" \
 gdb_trace_setactions "5.7: invalid action" \
 	"$trcpt1" \
 	"print gdb_c_test" \
-	"warning: .print gdb_c_test. is not a supported trace"
+	"'print gdb_c_test' is not a supported tracepoint action"
 
 # 5.8 help actions (collect, while-stepping, end)
 
@@ -230,13 +221,10 @@ gdb_trace_setactions "5.10a: set teval action for second tracepoint" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+teval gdb_char_test.
+\[\t \]+teval gdb_char_test.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+teval \\\$tsv \\\+= 1.
+\[\t \]+teval \\\$tsv \\\+= 1.
-\[\t \]+A\[\t \]+end.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_long_test.
+\[\t \]+collect gdb_long_test." \
-\[\t \]+A\[\t \]+end." \
 		"5.10a: verify teval actions set for two tracepoints"
 

gdb/testsuite/gdb.trace/while-stepping.exp

@@ -71,15 +71,9 @@ gdb_trace_setactions "5.12: set stepcount to $stepcount" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+step count 12 .*" \
+\[\t \]+while-stepping 12.*" \
-	"5.12: confirm stepcount set to $stepcount"
-
-gdb_test "info tracepoints" \
-    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-.*while-stepping $stepcount.*" \
 	"5.12: info trace shows \"while-stepping\""
 
-
 # 5.13 step out of context while collecting local variable
 #      [deferred to dynamic test section]
 
@@ -88,7 +82,7 @@ proc while_stepping_bogus_arg { bogus msgstring } {
 
     gdb_trace_setactions "$msgstring" \
 	    "" \
-	    "while-stepping $bogus" "\[Ee\]rror|\[Ww\]arning"
+	    "while-stepping $bogus" ".*bad step-count"
 }
 
 # 5.14 while-stepping (no argument)
@@ -109,7 +103,6 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 .*while-stepping $stepcount.*
-.*end.*
 .*end.*" \
 	"5.16: confirm actions, step without collecting anything"
 

gdb/tracepoint.c

@@ -178,7 +178,6 @@ static void add_aexpr (struct collection_list *, struct agent_expr *);
 static char *mem2hex (gdb_byte *, char *, int);
 static void add_register (struct collection_list *collection,
 			  unsigned int regno);
-static struct cleanup *make_cleanup_free_actions (struct breakpoint *t);
 
 extern void send_disconnected_tracing_value (int value);
 
@@ -450,9 +449,6 @@ tvariables_info (char *args, int from_tty)
 
 /* ACTIONS functions: */
 
-/* Prototypes for action-parsing utility commands  */
-static void read_actions (struct breakpoint *);
-
 /* The three functions:
    collect_pseudocommand, 
    while_stepping_pseudocommand, and 
@@ -491,138 +487,23 @@ static void
 trace_actions_command (char *args, int from_tty)
 {
   struct breakpoint *t;
-  char tmpbuf[128];
+  struct command_line *l;
-  char *end_msg = "End with a line saying just \"end\".";
 
   t = get_tracepoint_by_number (&args, 0, 1);
   if (t)
     {
-      sprintf (tmpbuf, "Enter actions for tracepoint %d, one per line.",
+      char *tmpbuf =
+	xstrprintf ("Enter actions for tracepoint %d, one per line.",
 		    t->number);
+      struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
 
-      if (from_tty)
+      l = read_command_lines (tmpbuf, from_tty, 1, check_tracepoint_command, t);
-	{
+      do_cleanups (cleanups);
-	  if (deprecated_readline_begin_hook)
+      breakpoint_set_commands (t, l);
-	    (*deprecated_readline_begin_hook) ("%s  %s\n", tmpbuf, end_msg);
-	  else if (input_from_terminal_p ())
-	    printf_filtered ("%s\n%s\n", tmpbuf, end_msg);
-	}
-
-      free_actions (t);
-      t->step_count = 0;	/* read_actions may set this */
-      read_actions (t);
-
-      if (deprecated_readline_end_hook)
-	(*deprecated_readline_end_hook) ();
-      /* tracepoints_changed () */
     }
   /* else just return */
 }
 
-/* worker function */
-static void
-read_actions (struct breakpoint *t)
-{
-  char *line;
-  char *prompt1 = "> ", *prompt2 = "  > ";
-  char *prompt = prompt1;
-  enum actionline_type linetype;
-  extern FILE *instream;
-  struct action_line *next = NULL, *temp;
-  struct cleanup *old_chain;
-
-  /* Control-C quits instantly if typed while in this loop
-     since it should not wait until the user types a newline.  */
-  immediate_quit++;
-  /* FIXME: kettenis/20010823: Something is wrong here.  In this file
-     STOP_SIGNAL is never defined.  So this code has been left out, at
-     least for quite a while now.  Replacing STOP_SIGNAL with SIGTSTP
-     leads to compilation failures since the variable job_control
-     isn't declared.  Leave this alone for now.  */
-#ifdef STOP_SIGNAL
-  if (job_control)
-    signal (STOP_SIGNAL, handle_stop_sig);
-#endif
-  old_chain = make_cleanup_free_actions (t);
-  while (1)
-    {
-      /* Make sure that all output has been output.  Some machines may
-         let you get away with leaving out some of the gdb_flush, but
-         not all.  */
-      wrap_here ("");
-      gdb_flush (gdb_stdout);
-      gdb_flush (gdb_stderr);
-
-      if (deprecated_readline_hook && instream == NULL)
-	line = (*deprecated_readline_hook) (prompt);
-      else if (instream == stdin && ISATTY (instream))
-	{
-	  line = gdb_readline_wrapper (prompt);
-	  if (line && *line)	/* add it to command history */
-	    add_history (line);
-	}
-      else
-	line = gdb_readline (0);
-
-      if (!line)
-        {
-          line = xstrdup ("end");
-          printf_filtered ("end\n");
-        }
-      
-      linetype = validate_actionline (&line, t);
-      if (linetype == BADLINE)
-	continue;		/* already warned -- collect another line */
-
-      temp = xmalloc (sizeof (struct action_line));
-      temp->next = NULL;
-      temp->action = line;
-
-      if (next == NULL)		/* first action for this tracepoint? */
-	t->actions = next = temp;
-      else
-	{
-	  next->next = temp;
-	  next = temp;
-	}
-
-      if (linetype == STEPPING)	/* begin "while-stepping" */
-	{
-	  if (prompt == prompt2)
-	    {
-	      warning (_("Already processing 'while-stepping'"));
-	      continue;
-	    }
-	  else
-	    prompt = prompt2;	/* change prompt for stepping actions */
-	}
-      else if (linetype == END)
-	{
-	  if (prompt == prompt2)
-	    {
-	      prompt = prompt1;	/* end of single-stepping actions */
-	    }
-	  else
-	    {			/* end of actions */
-	      if (t->actions->next == NULL)
-		{
-		  /* An "end" all by itself with no other actions
-		     means this tracepoint has no actions.
-		     Discard empty list.  */
-		  free_actions (t);
-		}
-	      break;
-	    }
-	}
-    }
-#ifdef STOP_SIGNAL
-  if (job_control)
-    signal (STOP_SIGNAL, SIG_DFL);
-#endif
-  immediate_quit--;
-  discard_cleanups (old_chain);
-}
-
 /* worker function */
 enum actionline_type
 validate_actionline (char **line, struct breakpoint *t)
@@ -772,8 +653,7 @@ validate_actionline (char **line, struct breakpoint *t)
       if (*p == '\0' ||
 	  (t->step_count = strtol (p, &p, 0)) == 0)
 	{
-	  warning (_("'%s': bad step-count; command ignored."), *line);
+	  error (_("'%s': bad step-count."), *line);
-	  return BADLINE;
 	}
       return STEPPING;
     }
@@ -781,39 +661,10 @@ validate_actionline (char **line, struct breakpoint *t)
     return END;
   else
     {
-      warning (_("'%s' is not a supported tracepoint action."), *line);
+      error (_("'%s' is not a supported tracepoint action."), *line);
-      return BADLINE;
     }
 }
 
-/* worker function */
-void
-free_actions (struct breakpoint *t)
-{
-  struct action_line *line, *next;
-
-  for (line = t->actions; line; line = next)
-    {
-      next = line->next;
-      if (line->action)
-	xfree (line->action);
-      xfree (line);
-    }
-  t->actions = NULL;
-}
-
-static void
-do_free_actions_cleanup (void *t)
-{
-  free_actions (t);
-}
-
-static struct cleanup *
-make_cleanup_free_actions (struct breakpoint *t)
-{
-  return make_cleanup (do_free_actions_cleanup, t);
-}
-
 enum {
   memrange_absolute = -1
 };
@@ -1271,70 +1122,31 @@ stringify_collection_list (struct collection_list *list, char *string)
     return *str_list;
 }
 
-/* Render all actions into gdb protocol.  */
+
-/*static*/ void
+static void
-encode_actions (struct breakpoint *t, struct bp_location *tloc,
+encode_actions_1 (struct command_line *action,
-		char ***tdp_actions, char ***stepping_actions)
+		  struct breakpoint *t,
+		  struct bp_location *tloc,
+		  int frame_reg,
+		  LONGEST frame_offset,
+		  struct collection_list *collect,
+		  struct collection_list *stepping_list)
 {
-  static char tdp_buff[2048], step_buff[2048];
   char *action_exp;
   struct expression *exp = NULL;
-  struct action_line *action;
+  struct command_line *actions;
   int i;
   struct value *tempval;
-  struct collection_list *collect;
   struct cmd_list_element *cmd;
   struct agent_expr *aexpr;
-  int frame_reg;
-  LONGEST frame_offset;
-  char *default_collect_line = NULL;
-  struct action_line *default_collect_action = NULL;
-
-  clear_collection_list (&tracepoint_list);
-  clear_collection_list (&stepping_list);
-  collect = &tracepoint_list;
-
-  *tdp_actions = NULL;
-  *stepping_actions = NULL;
-
-  gdbarch_virtual_frame_pointer (t->gdbarch,
-				 tloc->address, &frame_reg, &frame_offset);
-
-  action = t->actions;
-
-  /* If there are default expressions to collect, make up a collect
-     action and prepend to the action list to encode.  Note that since
-     validation is per-tracepoint (local var "xyz" might be valid for
-     one tracepoint and not another, etc), we make up the action on
-     the fly, and don't cache it.  */
-  if (*default_collect)
-    {
-      char *line;
-      enum actionline_type linetype;
-
-      default_collect_line = xmalloc (12 + strlen (default_collect));
-      sprintf (default_collect_line, "collect %s", default_collect);
-      line = default_collect_line;
-      linetype = validate_actionline (&line, t);
-      if (linetype != BADLINE)
-	{
-	  default_collect_action = xmalloc (sizeof (struct action_line));
-	  default_collect_action->next = t->actions;
-	  default_collect_action->action = line;
-	  action = default_collect_action;
-	}
-    }
 
   for (; action; action = action->next)
     {
       QUIT;			/* allow user to bail out with ^C */
-      action_exp = action->action;
+      action_exp = action->line;
       while (isspace ((int) *action_exp))
 	action_exp++;
 
-      if (*action_exp == '#')	/* comment line */
-	return;
-
       cmd = lookup_cmd (&action_exp, cmdlist, "", -1, 1);
       if (cmd == 0)
 	error (_("Bad action list item: %s"), action_exp);
@@ -1505,16 +1317,72 @@ encode_actions (struct breakpoint *t, struct bp_location *tloc,
 	}			/* if */
       else if (cmd_cfunc_eq (cmd, while_stepping_pseudocommand))
 	{
-	  collect = &stepping_list;
+	  /* We check against nested while-stepping when setting
+	     breakpoint action, so no way to run into nested
+	     here.  */
+	  gdb_assert (stepping_list);
+
+	  encode_actions_1 (action->body_list[0], t, tloc, frame_reg, frame_offset,
+			    stepping_list, NULL);
 	}
-      else if (cmd_cfunc_eq (cmd, end_actions_pseudocommand))
-	{
-	  if (collect == &stepping_list)	/* end stepping actions */
-	    collect = &tracepoint_list;
       else
-	    break;		/* end tracepoint actions */
+	error (_("Invalid tracepoint command '%s'"), action->line);
-	}
     }				/* for */
+}
+
+/* Render all actions into gdb protocol.  */
+/*static*/ void
+encode_actions (struct breakpoint *t, struct bp_location *tloc,
+		char ***tdp_actions, char ***stepping_actions)
+{
+  static char tdp_buff[2048], step_buff[2048];
+  char *default_collect_line = NULL;
+  struct command_line *actions;
+  struct command_line *default_collect_action = NULL;
+  int frame_reg;
+  LONGEST frame_offset;
+  struct cleanup *back_to;
+
+  back_to = make_cleanup (null_cleanup, NULL);
+
+  clear_collection_list (&tracepoint_list);
+  clear_collection_list (&stepping_list);
+
+  *tdp_actions = NULL;
+  *stepping_actions = NULL;
+
+  gdbarch_virtual_frame_pointer (t->gdbarch,
+				 t->loc->address, &frame_reg, &frame_offset);
+
+  actions = t->commands;
+
+  /* If there are default expressions to collect, make up a collect
+     action and prepend to the action list to encode.  Note that since
+     validation is per-tracepoint (local var "xyz" might be valid for
+     one tracepoint and not another, etc), we make up the action on
+     the fly, and don't cache it.  */
+  if (*default_collect)
+    {
+      char *line;
+      enum actionline_type linetype;
+
+      default_collect_line =  xstrprintf ("collect %s", default_collect);
+      make_cleanup (xfree, default_collect_line);
+
+      line = default_collect_line;
+      linetype = validate_actionline (&line, t);
+      if (linetype != BADLINE)
+	{
+	  default_collect_action = xmalloc (sizeof (struct command_line));
+	  make_cleanup (xfree, default_collect_action);
+	  default_collect_action->next = t->commands;
+	  default_collect_action->line = line;
+	  actions = default_collect_action;
+	}
+    }
+  encode_actions_1 (actions, t, tloc, frame_reg, frame_offset,
+		    &tracepoint_list, &stepping_list);
+
   memrange_sortmerge (&tracepoint_list);
   memrange_sortmerge (&stepping_list);
 
@@ -1523,8 +1391,7 @@ encode_actions (struct breakpoint *t, struct bp_location *tloc,
   *stepping_actions = stringify_collection_list (&stepping_list,
 						 step_buff);
 
-  xfree (default_collect_line);
+  do_cleanups (back_to);
-  xfree (default_collect_action);
 }
 
 static void
@@ -2276,7 +2143,7 @@ trace_dump_command (char *args, int from_tty)
   struct regcache *regcache;
   struct gdbarch *gdbarch;
   struct breakpoint *t;
-  struct action_line *action;
+  struct command_line *action;
   char *action_exp, *next_comma;
   struct cleanup *old_cleanups;
   int stepping_actions = 0;
@@ -2316,12 +2183,12 @@ trace_dump_command (char *args, int from_tty)
     if (loc->address == regcache_read_pc (regcache))
       stepping_frame = 0;
 
-  for (action = t->actions; action; action = action->next)
+  for (action = t->commands; action; action = action->next)
     {
       struct cmd_list_element *cmd;
 
       QUIT;			/* allow user to bail out with ^C */
-      action_exp = action->action;
+      action_exp = action->line;
       while (isspace ((int) *action_exp))
 	action_exp++;
 

gdb/tracepoint.h

@@ -20,13 +20,6 @@
 #if !defined (TRACEPOINT_H)
 #define TRACEPOINT_H 1
 
-/* The data structure for an action: */
-struct action_line
-  {
-    struct action_line *next;
-    char *action;
-  };
-
 enum actionline_type
   {
     BADLINE = -1,