RFC: "commands" command in user defined command

RFC: "commands" command in user defined command

[Jackie Smith Cashion]

In the current version of GDB you cannot put a "commands" command in a
user defined command. The "end" command intended to terminate the
"commands" command actually terminates the "define" command. The patches
needed to cause "commands" to be treated like "if" and "while" allowing
nested "end" processing are quite simple.

However, a more complicated problem is encountered when executing the
macro containing the "commands" command.

When executing a "commands" command GDB calls the function
commands_command. The job of commands_command is to collect the list of
commands that occur between the "commands" line and the "end" line into
a command_line structure and hang it off the associated breakpoint
structure. The commands_command function does not execute the list of
commands; this occurs later when the breakpoint is encountered.

In its current form commands_command() assumes that it must read the
list of commands from the input terminal. Until now that has always been
the correct action. The "commands" command list could only be entered
interactively. However, if we implement support for "commands" in a user
defined command, the list of commands may have already been read by
define_command and reside in a command_line structure hanging off the
user command definition.

Initially, it would appear that the commands_command function could be
modified so that when GDB is executing a "commands" command contained in
a user defined command it would not try to read the list of commands
from the terminal, but would simply use the list already hanging off the
user command structure. However, commands_command() does not have access
to the command_line structure in the user command definition. The only
arguments passed in are the "commands [arg]" command line string and the
from_tty flag. It is tempting to try to "hide" the pointer to the needed
command_line structure in one of these two arguments, but that is almost
certainly a bad design decision.

I feel that a better solution is to create a "sister" function
user_commands_command. This function is almost a copy of
commands_command except that instead of reading the command list
interactively, it receives a pointer to the command list in an argument.

Typically, a command contained in a macro is executed when
execute_user_command ultimately calls execute_command. execute_command
invokes the command function through the command vector table. All
functions entered in the vector table have an argument list like
commands_command, char * command and int from_tty.

Unfortunately, because of its non-standard argument,
user_commands_command cannot be invoked by calling execute_command.
execute_control_command (called by execute_user_command) has to call
user_commands_command directly. This means that the other logic in
execute_command will not be processed. The major loss is that the
pre-hook/post-hook logic will be bypassed.

I have attached a diff/patch file showing my proposed changes. (Please
ignore the "JSC" comments. These are temporary markers in my development
tree. I will remove them before submitting final patches.)

I welcome your comments.

Jackie Smith Cashion

--
? diff
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.52
diff -u -r1.52 breakpoint.c
--- breakpoint.c	2001/08/02 11:58:28	1.52
+++ breakpoint.c	2001/08/10 20:22:27
@@ -562,6 +562,8 @@
   error ("No breakpoint number %d.", bnum);
 }
 
+// JSC
+/* When modifying commands_command, update user_commands_command similarly. */
 /* ARGSUSED */
 static void
 commands_command (char *arg, int from_tty)
@@ -594,6 +596,55 @@
       l = read_command_lines (tmpbuf, from_tty);
       free_command_lines (&b->commands);
       b->commands = l;
+      breakpoints_changed ();
+      return;
+    }
+  error ("No breakpoint number %d.", bnum);
+}
+
+// JSC
+/* When modifying user_commands_command it may be appropriate to update
+   commands_command similarly. */
+
+/* This function is called to execute a "commands" command contained in
+   a user defined command. It is similar to the commands_command function.
+   However, the command list is not read interactively; it is pointed to
+   by the cmd argument.
+
+   user_commands_command is called directly by execute_control_command to
+   allow the command_line structure pointer (cmd) to be passed in. When
+   processing commands other than the "commands" command,
+   execute_control_command calls execute_command which, in turn, calls
+   the command function. However, this method does not provide access to the
+   command_line structure. The trade off for not going through execute_command
+   is that the pre-hook/post-hook logic is bypassed. */
+
+void
+user_commands_command (char *arg, struct command_line *cmd)
+{
+  register struct breakpoint *b;
+  char *p;
+  register int bnum;
+  struct command_line *l;
+
+  /* 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.");
+
+  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);
+      b->commands = cmd;
       breakpoints_changed ();
       return;
     }
Index: gdb/defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.62
diff -u -r1.62 defs.h
--- defs.h	2001/08/02 20:57:19	1.62
+++ defs.h	2001/08/10 20:22:28
@@ -819,6 +819,7 @@
     continue_control,
     while_control,
     if_control,
+    commands_control, // JSC
     invalid_control
   };
 
Index: gdb/cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.7
diff -u -r1.7 cli-script.c
--- cli-script.c	2001/06/17 15:16:12	1.7
+++ cli-script.c	2001/08/10 20:22:30
@@ -22,6 +22,7 @@
 #include "defs.h"
 #include "value.h"
 #include "language.h"		/* For value_true */
+#include "completer.h"		/* For line_completion_function */ // JSC
 #include <ctype.h>
 
 #ifdef UI_OUT
@@ -39,6 +40,10 @@
 
 extern void do_restore_instream_cleanup (void *stream);
 
+/* From gdb/breakpoint.c */                                        // JSC
+
+extern void user_commands_command (char *, struct command_line *); // JSC
+
 /* Prototypes for local functions */
 
 static struct cleanup *
@@ -76,14 +81,14 @@
 
 \f
 /* Allocate, initialize a new command line structure for one of the
-   control commands (if/while).  */
+   control commands (if/while/commands).  */ // JSC
 
 static struct command_line *
 build_command_line (enum command_control_type type, char *args)
 {
   struct command_line *cmd;
 
-  if (args == NULL)
+  if (type != commands_control && args == NULL) // JSC
     error ("if/while commands require arguments.\n");
 
   cmd = (struct command_line *) xmalloc (sizeof (struct command_line));
@@ -183,6 +188,22 @@
 	  continue;
 	}
 
+      // JSC
+      /* A commands command. Recursively print its subcommands and continue.
+	 The entire "commands" command line is in list->line, not just the
+	 optional argument. */
+      if (list->control_type == commands_control)
+	{
+	  ui_out_field_string (uiout, NULL, list->line);
+	  ui_out_text (uiout, "\n");
+	  print_command_lines (uiout, *list->body_list, depth + 1);
+	  if (depth)
+	    ui_out_spaces (uiout, 2 * depth);
+	  ui_out_text (uiout, "end\n");
+	  list = list->next;
+	  continue;
+	}
+
       /* An if command.  Recursively print both arms before continueing.  */
       if (list->control_type == if_control)
 	{
@@ -352,6 +373,27 @@
       ret = cmd->control_type;
       break;
 
+    // JSC
+    case commands_control:
+      /* Parse the "commands" command line to locate the optional breakpoint
+	 number argument. Skip any leading whitespace. Skip the command verb.
+	 Skip the white space between the verb and the argument. If there is
+	 no argument, pass a NULL pointer instead of an empty string. */
+      new_line = cmd->line;
+      if (!new_line)
+	return invalid_control;
+      while (*new_line == ' ' || *new_line == '\t')
+	new_line++;
+      while (*new_line != '\0' && *new_line != ' ' && *new_line != '\t')
+	new_line ++;
+      while (*new_line == ' ' || *new_line == '\t')
+	new_line++;
+      if (*new_line == '\0')
+	new_line = NULL;
+      user_commands_command (new_line, *cmd->body_list);
+      ret = simple_control;
+      break;
+
     case continue_control:
     case break_control:
       /* Return for "continue", and "break" so we can either
@@ -708,6 +750,48 @@
   command->body_count = new_length;
 }
 
+// JSC
+/* Return 1 (true) if the command line contains a "commands" command or an
+   unambiguous abbreviation; otherwise, return 0 (false). A "commands" command
+   has been encountered if the first call to line_completion_function returns
+   "commands" and the second call returns NULL indicating no other possible
+   completions. */
+
+static int
+is_commands_command (char *cmd)
+{
+  char *cmd_start, *cmd_end, *completion;
+  int len;
+
+  /* Isolate the command (the first word) in the command line.
+     Skip any leading white space. */
+  cmd_start = cmd;
+  while (*cmd_start == ' ' || *cmd_start == '\t')
+    cmd_start++;
+  cmd_end = cmd_start;
+  while (*cmd_end != '\0' && *cmd_end != ' ' && *cmd_end != '\t')
+    cmd_end++;
+
+  /* Empty command line. */
+  if (cmd_start == cmd_end)
+    return 0; /* false */
+      
+  len = cmd_end - cmd_start;
+  completion = line_completion_function (cmd_start, 0, cmd_start, len);
+  if (completion != NULL)
+    {
+      if (!strcmp (completion, "commands"))
+	{
+	  xfree (completion);
+	  completion = line_completion_function (cmd_start, 1, cmd_start, len);
+	  if (completion == NULL)
+	    return 1; /* true */
+	}
+      xfree (completion);
+    }
+  return 0; /* false */
+}
+
 /* Read one line from the input stream.  If the command is an "else" or
    "end", return such an indication to the caller.  */
 
@@ -752,7 +836,8 @@
   if (p1 == p || p[0] == '#')
     return nop_command;
 
-  /* Is this the end of a simple, while, or if control structure?  */
+  // JSC
+  /* Is this the end of a simple, while, if, or commands control structure?  */
   if (p1 - p == 3 && !strncmp (p, "end", 3))
     return end_command;
 
@@ -766,6 +851,10 @@
     *command = build_command_line (while_control, p + 6);
   else if (p1 - p > 2 && !strncmp (p, "if", 2))
     *command = build_command_line (if_control, p + 3);
+  else if (is_commands_command (p))                               // JSC
+    /* "commands" command found. Pass the entire command line,
+       not just the optional argument to build_command_line. */
+    *command = build_command_line (commands_control, p);
   else if (p1 - p == 10 && !strncmp (p, "loop_break", 10))
     {
       *command = (struct command_line *)
@@ -847,7 +936,8 @@
       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) // JSC
 	    {
 	      /* Success reading an entire control structure.  */
 	      ret = simple_control;
@@ -897,7 +987,8 @@
       /* 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) // JSC
 	{
 	  control_level++;
 	  ret = recurse_read_control_structure (next);
@@ -964,7 +1055,8 @@
 	}
 
       if (next->control_type == while_control
-	  || next->control_type == if_control)
+	  || next->control_type == if_control
+	  || next->control_type == commands_control) // JSC
 	{
 	  control_level++;
 	  ret = recurse_read_control_structure (next);
From eliz@is.elta.co.il Tue Aug 14 00:04:00 2001
From: "Eli Zaretskii" <eliz@is.elta.co.il>
To: ac131313@cygnus.com
Cc: bje@redhat.com, keiths@cygnus.com, kettenis@science.uva.nl, gdb-patches@sources.redhat.com
Subject: Re: [RFC] sigsetjmp/siglongjmp on cygwin
Date: Tue, 14 Aug 2001 00:04:00 -0000
Message-id: <2427-Tue14Aug2001100348+0300-eliz@is.elta.co.il>
References: <Pine.GSO.4.33.0108011357170.26158-100000@makita.cygnus.com> <3B738C62.6090607@cygnus.com> <15223.1933.710748.364749@scooby.brisbane.redhat.com> <2593-Mon13Aug2001120249+0300-eliz@is.elta.co.il> <3B7857A8.3040708@cygnus.com>
X-SW-Source: 2001-08/msg00163.html
Content-length: 884

> Date: Mon, 13 Aug 2001 18:41:44 -0400
> From: Andrew Cagney <ac131313@cygnus.com>
> 
> > I suggested to put the work-around into config.site, precisely to
> > solve this.  Assuming such a config.site is made part of the core
> > Cygwin distribution, whenever sigsetjmp starts working, config.site
> > should be updated.  So a user which upgrades to a newer version of
> > Cygwin has this issue magically taken care of during the upgrade.
> 
> Eli, can you expand a little.  I'm not sure what would happen if a 
> current GDB was configured/built on an old cygwin which didn't have the 
> config.site file.

Probably a moot point, now that the issue is closed, but here goes
anyway: the way to solve this is to put a config.site file into the
GDB source distribution (in some Cygwin-specific directory) and tell
the users to use that if they don't have config.site on their system.

Re: RFC: "commands" command in user defined command

[Eli Zaretskii]

> Date: Mon, 13 Aug 2001 22:33:09 -0500
> From: Jackie Smith Cashion <jsmith@redhat.com>
> 
> I feel that a better solution is to create a "sister" function
> user_commands_command. This function is almost a copy of
> commands_command except that instead of reading the command list
> interactively, it receives a pointer to the command list in an argument.

I don't have an opinion on this proposal, but if it gets approved,
please remember to update the user manual accordingly.