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

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
Consider the following snippet from a .gdbinit file:

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

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

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

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

The second case produces a complaint about "end":

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

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

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

The patches below fix this problem.

Okay to commit?

2006-01-13  Eli Zaretskii  <[hidden email]>

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

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

        * breakpoint.h (commands_from_control_command): Add prototype.

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


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

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

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

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

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

Daniel Jacobowitz-2
On Fri, Jan 13, 2006 at 04:48:37PM +0200, Eli Zaretskii wrote:
> The patches below fix this problem.
>
> Okay to commit?

Well, it looks good to me.  I assume this doesn't need any
documentation changes, but could you add it to the existing tests for
breakpoint commands, in addition?

Thanks for fixing this!

--
Daniel Jacobowitz
CodeSourcery
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> Date: Fri, 13 Jan 2006 10:32:13 -0500
> From: Daniel Jacobowitz <[hidden email]>
> Cc: [hidden email]
>
> On Fri, Jan 13, 2006 at 04:48:37PM +0200, Eli Zaretskii wrote:
> > The patches below fix this problem.
> >
> > Okay to commit?
>
> Well, it looks good to me.

Thanks for a prompt review.

> I assume this doesn't need any documentation changes

It doesn't, the documentation never mentioned any such limitation.

> but could you add it to the existing tests for breakpoint commands,
> in addition?

You mean in gdb.base/commands.exp?  I suppose I could do that, but I
don't have a test suite set up anywhere on the machines to which I
have access, so it might not be easy for me to test the tests I add.
Reply | Threaded
Open this post in threaded view
|

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

Daniel Jacobowitz-2
On Fri, Jan 13, 2006 at 06:02:35PM +0200, Eli Zaretskii wrote:
> You mean in gdb.base/commands.exp?  I suppose I could do that, but I
> don't have a test suite set up anywhere on the machines to which I
> have access, so it might not be easy for me to test the tests I add.

That's very unfortunate - it's standard practice to insist that code
patches be tested by the testsuite.  But if you can't do this, would
you mind drafting some tests, and then I'll make sure they work and
commit them?

--
Daniel Jacobowitz
CodeSourcery
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> Date: Sat, 14 Jan 2006 10:49:09 -0500
> From: Daniel Jacobowitz <[hidden email]>
>
> On Fri, Jan 13, 2006 at 06:02:35PM +0200, Eli Zaretskii wrote:
> > You mean in gdb.base/commands.exp?  I suppose I could do that, but I
> > don't have a test suite set up anywhere on the machines to which I
> > have access, so it might not be easy for me to test the tests I add.
>
> That's very unfortunate - it's standard practice to insist that code
> patches be tested by the testsuite.

Yes, I know.  Any pointers to howto set up a test suite from scratch
with minimal pain (on a Debian box)?  I might give it a try on some
rainy day.

> But if you can't do this, would you mind drafting some tests, and
> then I'll make sure they work and commit them?

Will do.  Thanks for the offer.
Reply | Threaded
Open this post in threaded view
|

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

Daniel Jacobowitz-2
On Sat, Jan 14, 2006 at 06:23:38PM +0200, Eli Zaretskii wrote:

> > Date: Sat, 14 Jan 2006 10:49:09 -0500
> > From: Daniel Jacobowitz <[hidden email]>
> >
> > On Fri, Jan 13, 2006 at 06:02:35PM +0200, Eli Zaretskii wrote:
> > > You mean in gdb.base/commands.exp?  I suppose I could do that, but I
> > > don't have a test suite set up anywhere on the machines to which I
> > > have access, so it might not be easy for me to test the tests I add.
> >
> > That's very unfortunate - it's standard practice to insist that code
> > patches be tested by the testsuite.
>
> Yes, I know.  Any pointers to howto set up a test suite from scratch
> with minimal pain (on a Debian box)?  I might give it a try on some
> rainy day.

Sure - it's trivial.  Just install the package "dejagnu" using apt,
and then run the testsuite using "make check-gdb" from the top level or
"make check" from the GDB subdirectory.

You will definitely get a handful of failures (which I'm not thrilled
about, but no longer have the energy to fight against).  So run it once
before changing anything and hold on to those baselines.  There's also
a small set of random failures, related to timing - interrupt tests and
thread tests.

--
Daniel Jacobowitz
CodeSourcery
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> Date: Sat, 14 Jan 2006 11:27:46 -0500
> From: Daniel Jacobowitz <[hidden email]>
>
> Just install the package "dejagnu" using apt,

Ehm.. apt will probably wish to install it in the public directories,
like /usr/local/bin etc., right?  I don't have sysadmin privileges on
that machine, so I need to configure and build it myself, so that it
will be installed in ~/bin.

Is fetching dejagnu from sourceware and building it all I will need?
Or are there additional packages I need to build first?
Reply | Threaded
Open this post in threaded view
|

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

Andreas Schwab
Eli Zaretskii <[hidden email]> writes:

> Is fetching dejagnu from sourceware and building it all I will need?
> Or are there additional packages I need to build first?

dejagnu depends on expect+tcl.

Andreas.

--
Andreas Schwab, SuSE Labs, [hidden email]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
In reply to this post by Eli Zaretskii
> Date: Fri, 13 Jan 2006 16:48:37 +0200
> From: Eli Zaretskii <[hidden email]>

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  <[hidden email]>

        * 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);
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> Date: Sat, 13 Jan 2007 19:11:41 +0200
> From: Eli Zaretskii <[hidden email]>
>
> (I will post an initial attempt to add a test for this in a moment.)

Here it is.  It is admittedly inelegant; in particular, the
diagnostics for a GDB that fails the test is not very smart, and one
of the two tests isn't counted in the sum of failed tests, although it
does fail.  Hopefully, someone more fluent with Expect will be able to
improve on the test (I think Daniel offered help back when I first
reported this).

--- testsuite/gdb.base/commands.exp.~0 2007-01-09 12:59:09.000000000 -0500
+++ testsuite/gdb.base/commands.exp 2007-01-13 12:00:18.912682122 -0500
@@ -618,6 +618,23 @@
     file delete file3
 }
 
+proc breakpoint_commands_in_if_command_test {} {
+    if [target_info exists noargs] {
+        verbose "Skipping breakpoint_commands_in_if_command_test because of noargs."
+        return
+    }
+
+    gdb_test "set \$tem = 1" "" "set \$tem in breakpoint_commands_in_if_command_test"
+    delete_breakpoints
+    gdb_test "if \$tem == 2\nbreak main\nelse\nbreak factorial\ncommands\nsilent\nset \$tem = 3\n\continue\nend\nend" \
+    "Breakpoint \[0-9\]+ at .*: file .*/run.c, line \[0-9\]+\." \
+    "breakpoint in else condition"
+    delete_breakpoints
+    gdb_test "if \$tem == 1\nbreak main\nelse\nbreak factorial\ncommands\nsilent\nset \$tem = 3\n\continue\nend\nend" \
+    "Breakpoint \[0-9\]+ at .*: file .*/run.c, line \[0-9\]+\." \
+    "breakpoint in if condition"
+}
+
 gdbvar_simple_if_test
 gdbvar_simple_while_test
 gdbvar_complex_if_while_test
@@ -635,3 +652,4 @@
 temporary_breakpoint_commands
 stray_arg0_test
 recursive_source_test
+breakpoint_commands_in_if_command_test
Reply | Threaded
Open this post in threaded view
|

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

Daniel Jacobowitz-2
In reply to this post by Eli Zaretskii
On Sat, Jan 13, 2007 at 07:11:41PM +0200, Eli Zaretskii wrote:
> Here's the renewed patch.  Okay to commit?

Looks OK to me.


--
Daniel Jacobowitz
CodeSourcery
Reply | Threaded
Open this post in threaded view
|

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

Daniel Jacobowitz-2
In reply to this post by Eli Zaretskii
On Sat, Jan 13, 2007 at 07:21:19PM +0200, Eli Zaretskii wrote:

> > Date: Sat, 13 Jan 2007 19:11:41 +0200
> > From: Eli Zaretskii <[hidden email]>
> >
> > (I will post an initial attempt to add a test for this in a moment.)
>
> Here it is.  It is admittedly inelegant; in particular, the
> diagnostics for a GDB that fails the test is not very smart, and one
> of the two tests isn't counted in the sum of failed tests, although it
> does fail.  Hopefully, someone more fluent with Expect will be able to
> improve on the test (I think Daniel offered help back when I first
> reported this).

I did indeed.  How does this look?

--
Daniel Jacobowitz
CodeSourcery

2007-01-20  Daniel Jacobowitz  <[hidden email]>

        * gdb.base/commands.exp: Call if_commands_test.
        (gdb_test_no_prompt, if_commands_test): New.

Index: testsuite/gdb.base/commands.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/commands.exp,v
retrieving revision 1.18
diff -u -p -r1.18 commands.exp
--- testsuite/gdb.base/commands.exp 9 Jan 2007 17:59:09 -0000 1.18
+++ testsuite/gdb.base/commands.exp 20 Jan 2007 20:32:44 -0000
@@ -618,6 +618,79 @@ end}
     file delete file3
 }
 
+proc gdb_test_no_prompt { command result msg } {
+    global gdb_prompt
+
+    set msg "$command - $msg"
+    set result "^[string_to_regexp $command]\r\n$result$"
+    gdb_test_multiple $command $msg {
+ -re "$result" {
+    pass $msg
+    return 1
+ }
+ -re "\r\n *>$" {
+    fail $msg
+    return 0
+ }
+    }
+    return 0
+}
+
+proc if_commands_test {} {
+    global gdb_prompt
+
+    gdb_test "set \$tem = 1" "" "set \$tem in if_commands_test"
+
+    set test "if_commands_test 1"
+    gdb_test_no_prompt "if \$tem == 2" { >} $test
+    gdb_test_no_prompt "break main" { >} $test
+    gdb_test_no_prompt "else" { >} $test
+    gdb_test_no_prompt "break factorial" { >} $test
+    gdb_test_no_prompt "commands" {  >} $test
+    gdb_test_no_prompt "silent" {  >} $test
+    gdb_test_no_prompt "set \$tem = 3" {  >} $test
+    gdb_test_no_prompt "continue" {  >} $test
+    gdb_test_multiple "end" "first end - $test" {
+ -re " >\$" {
+    pass "first end - $test"
+ }
+ -re "\r\n>\$" {
+    fail "first end - $test"
+ }
+    }
+    gdb_test_multiple "end" "second end - $test" {
+ -re "Breakpoint \[0-9\]+ at .*: file .*/run.c, line \[0-9\]+\.\r\n$gdb_prompt $" {
+    pass "second end - $test"
+ }
+ -re "Undefined command: \"silent\".*$gdb_prompt $" {
+    fail "second end - $test"
+ }
+    }
+
+    set test "if_commands_test 2"
+    gdb_test_no_prompt "if \$tem == 1" { >} $test
+    gdb_test_no_prompt "break main" { >} $test
+    gdb_test_no_prompt "else" { >} $test
+    gdb_test_no_prompt "break factorial" { >} $test
+    gdb_test_no_prompt "commands" {  >} $test
+    gdb_test_no_prompt "silent" {  >} $test
+    gdb_test_no_prompt "set \$tem = 3" {  >} $test
+    gdb_test_no_prompt "continue" {  >} $test
+    gdb_test_multiple "end" "first end - $test" {
+ -re " >\$" {
+    pass "first end - $test"
+ }
+ -re "\r\n>\$" {
+    fail "first end - $test"
+ }
+    }
+    gdb_test_multiple "end" "second end - $test" {
+ -re "Breakpoint \[0-9\]+ at .*: file .*/run.c, line \[0-9\]+\.\r\n$gdb_prompt $" {
+    pass "second end - $test"
+ }
+    }
+}
+
 gdbvar_simple_if_test
 gdbvar_simple_while_test
 gdbvar_complex_if_while_test
@@ -635,3 +708,4 @@ bp_deleted_in_command_test
 temporary_breakpoint_commands
 stray_arg0_test
 recursive_source_test
+if_commands_test
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> Date: Sat, 20 Jan 2007 15:33:52 -0500
> From: Daniel Jacobowitz <[hidden email]>
> Cc: [hidden email]
> >
> > Here it is.  It is admittedly inelegant; in particular, the
> > diagnostics for a GDB that fails the test is not very smart, and one
> > of the two tests isn't counted in the sum of failed tests, although it
> > does fail.  Hopefully, someone more fluent with Expect will be able to
> > improve on the test (I think Daniel offered help back when I first
> > reported this).
>
> I did indeed.  How does this look?

Looks fine, thanks.  With an unpatched GDB it fails:

    Running ./gdb.base/commands.exp ...
    FAIL: gdb.base/commands.exp: commands - if_commands_test 1
    FAIL: gdb.base/commands.exp: silent - if_commands_test 1
    FAIL: gdb.base/commands.exp: set $tem = 3 - if_commands_test 1
    FAIL: gdb.base/commands.exp: continue - if_commands_test 1
    FAIL: gdb.base/commands.exp: first end - if_commands_test 1
    FAIL: gdb.base/commands.exp: second end - if_commands_test 1
    FAIL: gdb.base/commands.exp: commands - if_commands_test 2
    FAIL: gdb.base/commands.exp: silent - if_commands_test 2
    FAIL: gdb.base/commands.exp: set $tem = 3 - if_commands_test 2
    FAIL: gdb.base/commands.exp: continue - if_commands_test 2
    FAIL: gdb.base/commands.exp: first end - if_commands_test 2
    FAIL: gdb.base/commands.exp: second end - if_commands_test 2

                    === gdb Summary ===

    # of expected passes            95
    # of unexpected failures        12

    make[1]: *** [just-check] Error 1

While after patching it succeeds:

    Running ./gdb.base/commands.exp ...
   
                    === gdb Summary ===
   
    # of expected passes            107
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
In reply to this post by Daniel Jacobowitz-2
> Date: Sat, 20 Jan 2007 15:04:17 -0500
> From: Daniel Jacobowitz <[hidden email]>
> Cc: [hidden email]
> X-Spam-Status: No, score=0.0 required=5.0 tests=none autolearn=failed
> version=3.0.4
>
> On Sat, Jan 13, 2007 at 07:11:41PM +0200, Eli Zaretskii wrote:
> > Here's the renewed patch.  Okay to commit?
>
> Looks OK to me.

Thanks, committed.
Reply | Threaded
Open this post in threaded view
|

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

Daniel Jacobowitz-2
In reply to this post by Eli Zaretskii
On Sat, Jan 27, 2007 at 02:24:09PM +0200, Eli Zaretskii wrote:
> Looks fine, thanks.

OK, I checked it in.

--
Daniel Jacobowitz
CodeSourcery