This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [MI tracepoints 2/9] unify breakpoint commands and tracepoint actions


On Monday 15 March 2010 20:47:49 Stan Shebs wrote:

> Vladimir Prus wrote:
> > There's a lot of similarity, conceptual and code-wise, between breakpoint
> > commands and tracepoint actions. This patch unifies both. OK?
> >   
> This deserves a NEWS entry, 

Added.

> in fact the new MI commands should have a 
> (separate) NEWS entry as well.  

Sorry, what new MI commands?

> Also, did I miss the changes to the 
> manual?  I'm not seeing them here or in patch 9/9.
> 
> OK with those additions.  Tom, is this going to interfere with your 
> multiple-breakpoints commands patch?  Should we do any coordination on that?

Here's a revised version, also addressing Pedro's comments.


Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722
diff --git a/gdb/NEWS b/gdb/NEWS
index 64c48f6..94e54ca 100644
--- a/gdb/NEWS
+++ b/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*
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8d0977f..19707e8 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -744,10 +744,10 @@ 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.  */
+      /* 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)
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 3788b6e..3f194c6 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1007,4 +1007,8 @@ 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) */
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 75dfd02..204565c 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -252,8 +252,6 @@ mi_cmd_break_commands (char *command, char **argv, int argc)
   int bnum;
   struct breakpoint *b;
 
-  extern void check_tracepoint_command (char *line, void *closure);
-
   if (argc < 1)
     error ("USAGE: %s <BKPT> [<COMMAND> [<COMMAND>...]]", command);
 
diff --git a/gdb/testsuite/gdb.trace/while-stepping.exp b/gdb/testsuite/gdb.trace/while-stepping.exp
index 6c156a1..7e422a5 100644
--- a/gdb/testsuite/gdb.trace/while-stepping.exp
+++ b/gdb/testsuite/gdb.trace/while-stepping.exp
@@ -74,13 +74,6 @@ gdb_test "info tracepoints" \
 \[\t \]+while-stepping 12.*" \
 	"5.12: info trace shows \"while-stepping\""
 
-
-#gdb_test "info tracepoints" \
-#    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-#.*while-stepping $stepcount.*" \
-
-
-
 # 5.13 step out of context while collecting local variable
 #      [deferred to dynamic test section]
 
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 38f41b7..c713d7e 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -484,8 +484,6 @@ trace_actions_command (char *args, int from_tty)
   struct breakpoint *t;
   struct command_line *l;
 
-  extern void check_tracepoint_command (char *line, void *closure);
-
   t = get_tracepoint_by_number (&args, 0, 1);
   if (t)
     {
commit 40ea0c466c395f63b64cab9b719ace03c8f31d77
Author: vladimir <vladimir@e7755896-6108-0410-9592-8049d3e74e28>
Date:   Thu Aug 6 07:50:11 2009 +0000

    Unify actions and commands
    
    Now, breakpoints do not have separate commands and actions list.
    Instead, when setting commands for a tracepoint, we just verify
    those commands are permissible. The 'actions' command is retained
    for backward compatibility, but I would suggest that it be
    removed for FSF submission. This patch means that parsing and
    printing of commands for tracepoints is identical to that for
    ordinary breakpoints. In particular, there's no 'A' printed,
    and the body of 'while-stepping' is indented just like while's.
    
    This patch also removes printing of breakpoint's step count,
    since it's fairly obvious from 'while-stepping' that is also
    printed. In fact, it does not seem that step count is intrinsic
    property of breakpoint, so ideally should be removed.
    However, find_matching_tracepoint uses step_count at present,
    so we need to update it to actually compare actions before
    removing breakpoint::step_count.
    
    	* 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): Adjust.
    	* 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/testsuite/
    	* gdb.trace/actions.exp: Adjust for output changes.
    	* gdb.trace/while-stepping.exp: Likewise.

diff --git a/gdb/NEWS b/gdb/NEWS
index 64c48f6..94e54ca 100644
--- a/gdb/NEWS
+++ b/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*
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1b4ab57..19707e8 100644
--- a/gdb/breakpoint.c
+++ b/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)
@@ -10315,7 +10392,7 @@ 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 *i1 = "    ", *i2 = "      ";
   char *indent, *actionline, *pathname;
@@ -10358,16 +10435,16 @@ tracepoint_save_command (char *args, int from_tty)
     if (tp->pass_count)
       fprintf (fp, "  passcount %d\n", tp->pass_count);
 
-    if (tp->actions)
+    if (tp->commands)
       {
 	fprintf (fp, "  actions\n");
 	indent = i1;
-	for (line = tp->actions; line; line = line->next)
+	for (line = tp->commands; line; line = line->next)
 	  {
 	    struct cmd_list_element *cmd;
 
 	    QUIT;		/* allow user to bail out with ^C */
-	    actionline = line->action;
+	    actionline = line->line;
 	    while (isspace ((int) *actionline))
 	      actionline++;
 
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 438cc6a..3f194c6 100644
--- a/gdb/breakpoint.h
+++ b/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) */
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 185de36..660ea46 100644
--- a/gdb/cli/cli-script.c
+++ b/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)
 	{
-	  ui_out_field_fmt (uiout, NULL, "while %s", list->line);
+	  /* 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);
diff --git a/gdb/defs.h b/gdb/defs.h
index 5983289..b7fd97a 100644
--- a/gdb/defs.h
+++ b/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_1 (char * (*) (void), int);
+extern struct command_line *read_command_lines (char *, int, 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 **);
 
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 3ed652c..204565c 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/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);
 }
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 01d558c..ed1dc20 100644
--- a/gdb/remote.c
+++ b/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); */
diff --git a/gdb/testsuite/gdb.trace/actions.exp b/gdb/testsuite/gdb.trace/actions.exp
index 7168501..05ab0a9 100644
--- a/gdb/testsuite/gdb.trace/actions.exp
+++ b/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 \]+A\[\t \]+end.
+\[\t \]+collect gdb_char_test.
 \[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 \]+A\[\t \]+end.
+\[\t \]+collect gdb_char_test.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_short_test.
-\[\t \]+A\[\t \]+end.
+\[\t \]+collect gdb_short_test.
 \[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 \]+A\[\t \]+end.
+\[\t \]+collect gdb_char_test.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_short_test.
-\[\t \]+A\[\t \]+end.
+\[\t \]+collect gdb_short_test.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_long_test.
-\[\t \]+A\[\t \]+end." \
+\[\t \]+collect gdb_long_test." \
 		"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 \]+A\[\t \]+end.
+\[\t \]+collect gdb_struct1_test.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_short_test.
-\[\t \]+A\[\t \]+end.
+\[\t \]+collect gdb_short_test.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+A\[\t \]+collect gdb_long_test.
-\[\t \]+A\[\t \]+end." \
+\[\t \]+collect gdb_long_test." \
 		"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)
 
diff --git a/gdb/testsuite/gdb.trace/while-stepping.exp b/gdb/testsuite/gdb.trace/while-stepping.exp
index c75ac4f..7e422a5 100644
--- a/gdb/testsuite/gdb.trace/while-stepping.exp
+++ b/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 .*" \
-	"5.12: confirm stepcount set to $stepcount"
-
-gdb_test "info tracepoints" \
-    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-.*while-stepping $stepcount.*" \
+\[\t \]+while-stepping 12.*" \
 	"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" "\[Ee\]rror|\[Ww\]arning|.*is malformed"
 }
 
 # 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"
 
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index e1679fa..c713d7e 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -173,7 +173,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);
 
@@ -445,9 +444,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 
@@ -486,139 +482,24 @@ static void
 trace_actions_command (char *args, int from_tty)
 {
   struct breakpoint *t;
-  char tmpbuf[128];
-  char *end_msg = "End with a line saying just \"end\".";
+  struct command_line *l;
 
   t = get_tracepoint_by_number (&args, 0, 1);
   if (t)
     {
-      sprintf (tmpbuf, "Enter actions for tracepoint %d, one per line.",
-	       t->number);
-
-      if (from_tty)
-	{
-	  if (deprecated_readline_begin_hook)
-	    (*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);
-	}
+      char *tmpbuf =
+	xstrprintf ("Enter actions for tracepoint %d, one per line.",
+		    t->number);
+      struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
 
-      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 () */
+      l = read_command_lines (tmpbuf, from_tty, 1, check_tracepoint_command, t);
+      do_cleanups (cleanups);
+      breakpoint_set_commands (t, l);
     }
   /* 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)
 {
@@ -767,8 +648,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);
-	  return BADLINE;
+	  error (_("'%s': bad step-count."), *line);
 	}
       return STEPPING;
     }
@@ -776,37 +656,8 @@ validate_actionline (char **line, struct breakpoint *t)
     return END;
   else
     {
-      warning (_("'%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);
+      error (_("'%s' is not a supported tracepoint action."), *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 {
@@ -1247,70 +1098,31 @@ stringify_collection_list (struct collection_list *list, char *string)
     return *str_list;
 }
 
-/* Render all actions into gdb protocol.  */
-/*static*/ void
-encode_actions (struct breakpoint *t, struct bp_location *tloc,
-		char ***tdp_actions, char ***stepping_actions)
+
+static void
+encode_actions_1 (struct command_line *action,
+		  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);
@@ -1481,26 +1293,81 @@ 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))
+      else
+	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)
 	{
-	  if (collect == &stepping_list)	/* end stepping actions */
-	    collect = &tracepoint_list;
-	  else
-	    break;		/* end tracepoint actions */
+	  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;
 	}
-    }				/* for */
+    }
+  encode_actions_1 (actions, t, tloc, frame_reg, frame_offset,
+		    &tracepoint_list, &stepping_list);
+
   memrange_sortmerge (&tracepoint_list);
   memrange_sortmerge (&stepping_list);
 
-  *tdp_actions = stringify_collection_list (&tracepoint_list, 
+  *tdp_actions = stringify_collection_list (&tracepoint_list,
 					    tdp_buff);
-  *stepping_actions = stringify_collection_list (&stepping_list, 
+  *stepping_actions = stringify_collection_list (&stepping_list,
 						 step_buff);
 
-  xfree (default_collect_line);
-  xfree (default_collect_action);
+  do_cleanups (back_to);
 }
 
 static void
@@ -2231,7 +2098,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;
@@ -2271,12 +2138,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++;
 
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 819a67a..446b292 100644
--- a/gdb/tracepoint.h
+++ b/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,

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]