Command line input handling TLC

I didn't manage to usefully split this further into smaller
independent pieces, so:

 - Use "struct buffer" more.

 - Split out the responsibility of composing a complete command line
   from multiple input lines split with backslash

    (
    E.g.:

       (gdb) print \
       1 + \
       2
       $1 = 3
       (gdb)
    )

   to a separate function.  Note we don't need the separate
   readline_input_state and more_to_come globals at all.  They were
   just obfuscating the logic.

 - Factor out the tricky mostly duplicated code in
   command_line_handler and command_line_input.

gdb/ChangeLog
2016-03-09  Pedro Alves  <palves@redhat.com>

	* event-top.c (more_to_come): Delete.
	(struct readline_input_state): Delete.
	(readline_input_state): Delete.
	(get_command_line_buffer): New function.
	(command_handler): Update comments.  Don't handle NULL commands
	here.  Do not execute commented lines.
	(command_line_append_input_line): New function.
	(handle_line_of_input): New function, partly based on
	command_line_handler and command_line_input.
	(command_line_handler): Rewrite.
	* event-top.h (command_handler): New declaration.
	(command_loop): Defer command execution to command_handler.
	(command_line_input): Update comments.  Simplify, using struct
	buffer and handle_line_of_input.
	* top.h (struct buffer): New forward declaration.
	(handle_line_of_input): New declaration.

gdb/ChangeLog

@@ -1,3 +1,22 @@
+2016-03-09  Pedro Alves  <palves@redhat.com>
+
+	* event-top.c (more_to_come): Delete.
+	(struct readline_input_state): Delete.
+	(readline_input_state): Delete.
+	(get_command_line_buffer): New function.
+	(command_handler): Update comments.  Don't handle NULL commands
+	here.  Do not execute commented lines.
+	(command_line_append_input_line): New function.
+	(handle_line_of_input): New function, partly based on
+	command_line_handler and command_line_input.
+	(command_line_handler): Rewrite.
+	* event-top.h (command_handler): New declaration.
+	(command_loop): Defer command execution to command_handler.
+	(command_line_input): Update comments.  Simplify, using struct
+	buffer and handle_line_of_input.
+	* top.h (struct buffer): New forward declaration.
+	(handle_line_of_input): New declaration.
+
 2016-03-09  Pedro Alves  <palves@redhat.com>
 
 	* event-top.c (command_line_handler): Use xfree + xstrdup instead

gdb/event-top.c

@@ -49,7 +49,6 @@
 static void rl_callback_read_char_wrapper (gdb_client_data client_data);
 static void command_line_handler (char *rl);
 static void change_line_handler (void);
-static void command_handler (char *command);
 static char *top_level_prompt (void);
 
 /* Signal handlers.  */
@@ -140,20 +139,6 @@ static struct async_signal_handler *sigtstp_token;
 #endif
 static struct async_signal_handler *async_sigterm_token;
 
-/* Structure to save a partially entered command.  This is used when
-   the user types '\' at the end of a command line.  This is necessary
-   because each line of input is handled by a different call to
-   command_line_handler, and normally there is no state retained
-   between different calls.  */
-static int more_to_come = 0;
-
-struct readline_input_state
-  {
-    char *linebuffer;
-    char *linebuffer_ptr;
-  }
-readline_input_state;
-
 /* This hook is called by rl_callback_read_char_wrapper after each
    character is processed.  */
 void (*after_char_processing_hook) (void);
@@ -383,6 +368,24 @@ top_level_prompt (void)
   return xstrdup (prompt);
 }
 
+/* Get a pointer to the command line buffer.  This is used to
+   construct a whole line of input from partial input.  */
+
+static struct buffer *
+get_command_line_buffer (void)
+{
+  static struct buffer line_buffer;
+  static int line_buffer_initialized;
+
+  if (!line_buffer_initialized)
+    {
+      buffer_init (&line_buffer);
+      line_buffer_initialized = 1;
+    }
+
+  return &line_buffer;
+}
+
 /* When there is an event ready on the stdin file descriptor, instead
    of calling readline directly throught the callback function, or
    instead of calling gdb_readline_no_editing_callback, give gdb a
@@ -436,152 +439,122 @@ async_disable_stdin (void)
 }
 
 
-/* Handles a gdb command.  This function is called by
+/* Handle a gdb command line.  This function is called when
-   command_line_handler, which has processed one or more input lines
+   handle_line_of_input has concatenated one or more input lines into
-   into COMMAND.  */
+   a whole command.  */
-/* NOTE: 1999-04-30 This is the asynchronous version of the command_loop
+
-   function.  The command_loop function will be obsolete when we
+void
-   switch to use the event loop at every execution of gdb.  */
-static void
 command_handler (char *command)
 {
   struct cleanup *stat_chain;
+  char *c;
 
   clear_quit_flag ();
   if (instream == stdin)
     reinitialize_more_filter ();
 
-  /* If readline returned a NULL command, it means that the connection
-     with the terminal is gone.  This happens at the end of a
-     testsuite run, after Expect has hung up but GDB is still alive.
-     In such a case, we just quit gdb killing the inferior program
-     too.  */
-  if (command == 0)
-    {
-      printf_unfiltered ("quit\n");
-      execute_command ("quit", stdin == instream);
-    }
-
   stat_chain = make_command_stats_cleanup (1);
 
+  /* Do not execute commented lines.  */
+  for (c = command; *c == ' ' || *c == '\t'; c++)
+    ;
+  if (c[0] != '#')
+    {
       execute_command (command, instream == stdin);
 
       /* Do any commands attached to breakpoint we stopped at.  */
       bpstat_do_actions ();
+    }
 
   do_cleanups (stat_chain);
 }
 
-/* Handle a complete line of input.  This is called by the callback
+/* Append RL, an input line returned by readline or one of its
-   mechanism within the readline library.  Deal with incomplete
+   emulations, to CMD_LINE_BUFFER.  Returns the command line if we
-   commands as well, by saving the partial input in a global
+   have a whole command line ready to be processed by the command
-   buffer.  */
+   interpreter or NULL if the command line isn't complete yet (input
+   line ends in a backslash).  Takes ownership of RL.  */
 
-/* NOTE: 1999-04-30 This is the asynchronous version of the
+static char *
-   command_line_input function; command_line_input will become
+command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
-   obsolete once we use the event loop as the default mechanism in
+{
-   GDB.  */
+  char *cmd;
-static void
+  size_t len;
-command_line_handler (char *rl)
+
+  len = strlen (rl);
+
+  if (len > 0 && rl[len - 1] == '\\')
+    {
+      /* Don't copy the backslash and wait for more.  */
+      buffer_grow (cmd_line_buffer, rl, len - 1);
+      cmd = NULL;
+    }
+  else
+    {
+      /* Copy whole line including terminating null, and we're
+	 done.  */
+      buffer_grow (cmd_line_buffer, rl, len + 1);
+      cmd = cmd_line_buffer->buffer;
+    }
+
+  /* Allocated in readline.  */
+  xfree (rl);
+
+  return cmd;
+}
+
+/* Handle a line of input coming from readline.
+
+   If the read line ends with a continuation character (backslash),
+   save the partial input in CMD_LINE_BUFFER (except the backslash),
+   and return NULL.  Otherwise, save the partial input and return a
+   pointer to CMD_LINE_BUFFER's buffer (null terminated), indicating a
+   whole command line is ready to be executed.
+
+   Returns EOF on end of file.
+
+   If REPEAT, handle command repetitions:
+
+     - If the input command line is NOT empty, the command returned is
+       copied into the global 'saved_command_line' var so that it can
+       be repeated later.
+
+     - OTOH, if the input command line IS empty, return the previously
+       saved command instead of the empty input line.
+*/
+
+char *
+handle_line_of_input (struct buffer *cmd_line_buffer,
+		      char *rl, int repeat, char *annotation_suffix)
 {
-  static char *linebuffer = 0;
-  static unsigned linelength = 0;
-  char *p;
   char *p1;
-  char *nline;
+  char *cmd;
-  int repeat = (instream == stdin);
+
+  if (rl == NULL)
+    return (char *) EOF;
+
+  cmd = command_line_append_input_line (cmd_line_buffer, rl);
+  if (cmd == NULL)
+    return NULL;
+
+  /* We have a complete command line now.  Prepare for the next
+     command, but leave ownership of memory to the buffer .  */
+  cmd_line_buffer->used_size = 0;
 
   if (annotation_level > 1 && instream == stdin)
-    printf_unfiltered (("\n\032\032post-prompt\n"));
-
-  if (linebuffer == 0)
     {
-      linelength = 80;
+      printf_unfiltered (("\n\032\032post-"));
-      linebuffer = (char *) xmalloc (linelength);
+      puts_unfiltered (annotation_suffix);
-      linebuffer[0] = '\0';
+      printf_unfiltered (("\n"));
     }
 
-  p = linebuffer;
+#define SERVER_COMMAND_PREFIX "server "
-
+  if (startswith (cmd, SERVER_COMMAND_PREFIX))
-  if (more_to_come)
     {
-      strcpy (linebuffer, readline_input_state.linebuffer);
+      /* Note that we don't set `saved_command_line'.  Between this
-      p = readline_input_state.linebuffer_ptr;
+         and the check in dont_repeat, this insures that repeating
-      xfree (readline_input_state.linebuffer);
+         will still do the right thing.  */
-      more_to_come = 0;
+      return cmd + strlen (SERVER_COMMAND_PREFIX);
-    }
-
-#ifdef STOP_SIGNAL
-  if (job_control)
-    signal (STOP_SIGNAL, handle_stop_sig);
-#endif
-
-  /* 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 (source_file_name != NULL)
-    ++source_line_number;
-
-  /* If we are in this case, then command_handler will call quit 
-     and exit from gdb.  */
-  if (!rl || rl == (char *) EOF)
-    {
-      command_handler (0);
-      return;			/* Lint.  */
-    }
-  if (strlen (rl) + 1 + (p - linebuffer) > linelength)
-    {
-      linelength = strlen (rl) + 1 + (p - linebuffer);
-      nline = (char *) xrealloc (linebuffer, linelength);
-      p += nline - linebuffer;
-      linebuffer = nline;
-    }
-  p1 = rl;
-  /* Copy line.  Don't copy null at end.  (Leaves line alone
-     if this was just a newline).  */
-  while (*p1)
-    *p++ = *p1++;
-
-  xfree (rl);			/* Allocated in readline.  */
-
-  if (p > linebuffer && *(p - 1) == '\\')
-    {
-      *p = '\0';
-      p--;			/* Put on top of '\'.  */
-
-      readline_input_state.linebuffer = xstrdup (linebuffer);
-      readline_input_state.linebuffer_ptr = p;
-
-      /* We will not invoke a execute_command if there is more
-	 input expected to complete the command.  So, we need to
-	 print an empty prompt here.  */
-      more_to_come = 1;
-      display_gdb_prompt ("");
-      return;
-    }
-
-#ifdef STOP_SIGNAL
-  if (job_control)
-    signal (STOP_SIGNAL, SIG_DFL);
-#endif
-
-#define SERVER_COMMAND_LENGTH 7
-  server_command =
-    (p - linebuffer > SERVER_COMMAND_LENGTH)
-    && strncmp (linebuffer, "server ", SERVER_COMMAND_LENGTH) == 0;
-  if (server_command)
-    {
-      /* Note that we don't set `line'.  Between this and the check in
-         dont_repeat, this insures that repeating will still do the
-         right thing.  */
-      *p = '\0';
-      command_handler (linebuffer + SERVER_COMMAND_LENGTH);
-      display_gdb_prompt (0);
-      return;
     }
 
   /* Do history expansion if that is wished.  */
@@ -591,10 +564,11 @@ command_line_handler (char *rl)
       char *history_value;
       int expanded;
 
-      *p = '\0';		/* Insert null now.  */
+      expanded = history_expand (cmd, &history_value);
-      expanded = history_expand (linebuffer, &history_value);
       if (expanded)
 	{
+	  size_t len;
+
 	  /* Print the changes.  */
 	  printf_unfiltered ("%s\n", history_value);
 
@@ -602,67 +576,81 @@ command_line_handler (char *rl)
 	  if (expanded < 0)
 	    {
 	      xfree (history_value);
-	      return;
+	      return cmd;
 	    }
-	  if (strlen (history_value) > linelength)
+
-	    {
+	  /* history_expand returns an allocated string.  Just replace
-	      linelength = strlen (history_value) + 1;
+	     our buffer with it.  */
-	      linebuffer = (char *) xrealloc (linebuffer, linelength);
+	  len = strlen (history_value);
+	  xfree (buffer_finish (cmd_line_buffer));
+	  cmd_line_buffer->buffer = history_value;
+	  cmd_line_buffer->buffer_size = len + 1;
+	  cmd = history_value;
 	}
-	  strcpy (linebuffer, history_value);
-	  p = linebuffer + strlen (linebuffer);
-	}
-      xfree (history_value);
     }
 
   /* If we just got an empty line, and that is supposed to repeat the
-     previous command, return the value in the global buffer.  */
+     previous command, return the previously saved command.  */
-  if (repeat && p == linebuffer && *p != '\\')
+  for (p1 = cmd; *p1 == ' ' || *p1 == '\t'; p1++)
-    {
+    ;
-      command_handler (saved_command_line);
+  if (repeat && *p1 == '\0')
-      display_gdb_prompt (0);
+    return saved_command_line;
-      return;
-    }
 
-  for (p1 = linebuffer; *p1 == ' ' || *p1 == '\t'; p1++);
+  /* Add command to history if appropriate.  Note: lines consisting
-  if (repeat && !*p1)
+     solely of comments are also added to the command history.  This
-    {
+     is useful when you type a command, and then realize you don't
-      command_handler (saved_command_line);
+     want to execute it quite yet.  You can comment out the command
-      display_gdb_prompt (0);
+     and then later fetch it from the value history and remove the
-      return;
+     '#'.  The kill ring is probably better, but some people are in
-    }
+     the habit of commenting things out.  */
-
+  if (*cmd != '\0' && input_from_terminal_p ())
-  *p = 0;
+    gdb_add_history (cmd);
-
-  /* Add line to history if appropriate.  */
-  if (*linebuffer && input_from_terminal_p ())
-    gdb_add_history (linebuffer);
-
-  /* Note: lines consisting solely of comments are added to the command
-     history.  This is useful when you type a command, and then
-     realize you don't want to execute it quite yet.  You can comment
-     out the command and then later fetch it from the value history
-     and remove the '#'.  The kill ring is probably better, but some
-     people are in the habit of commenting things out.  */
-  if (*p1 == '#')
-    *p1 = '\0';			/* Found a comment.  */
 
   /* Save into global buffer if appropriate.  */
   if (repeat)
     {
       xfree (saved_command_line);
-      saved_command_line = xstrdup (linebuffer);
+      saved_command_line = xstrdup (cmd);
-      if (!more_to_come)
+      return saved_command_line;
-	{
-	  command_handler (saved_command_line);
-	  display_gdb_prompt (0);
     }
-      return;
+  else
+    return cmd;
 }
 
-  command_handler (linebuffer);
+/* Handle a complete line of input.  This is called by the callback
+   mechanism within the readline library.  Deal with incomplete
+   commands as well, by saving the partial input in a global
+   buffer.
+
+   NOTE: This is the asynchronous version of the command_line_input
+   function.  */
+
+void
+command_line_handler (char *rl)
+{
+  struct buffer *line_buffer = get_command_line_buffer ();
+  char *cmd;
+
+  cmd = handle_line_of_input (line_buffer, rl, instream == stdin, "prompt");
+  if (cmd == (char *) EOF)
+    {
+      /* stdin closed.  The connection with the terminal is gone.
+	 This happens at the end of a testsuite run, after Expect has
+	 hung up but GDB is still alive.  In such a case, we just quit
+	 gdb killing the inferior program too.  */
+      printf_unfiltered ("quit\n");
+      execute_command ("quit", stdin == instream);
+    }
+  else if (cmd == NULL)
+    {
+      /* We don't have a full line yet.  Print an empty prompt.  */
+      display_gdb_prompt ("");
+    }
+  else
+    {
+      command_handler (cmd);
       display_gdb_prompt (0);
-  return;
+    }
 }
 
 /* Does reading of input from terminal w/o the editing features

gdb/event-top.h

@@ -34,6 +34,8 @@ extern void async_init_signals (void);
 extern void set_async_editing_command (char *args, int from_tty,
 				       struct cmd_list_element *c);
 
+extern void command_handler (char *command);
+
 /* Signal to catch ^Z typed while reading a command: SIGTSTP or SIGCONT.  */
 #ifndef STOP_SIGNAL
 #include <signal.h>

gdb/top.c

@@ -533,37 +533,17 @@ execute_command_to_string (char *p, int from_tty)
 void
 command_loop (void)
 {
-  struct cleanup *old_chain;
-  char *command;
-
   while (instream && !feof (instream))
     {
-      clear_quit_flag ();
+      char *command;
-      if (instream == stdin)
-	reinitialize_more_filter ();
-      old_chain = make_cleanup (null_cleanup, 0);
 
       /* Get a command-line.  This calls the readline package.  */
       command = command_line_input (instream == stdin ?
 				    get_prompt () : (char *) NULL,
 				    instream == stdin, "prompt");
-      if (command == 0)
+      if (command == NULL)
-	{
-	  do_cleanups (old_chain);
 	return;
-	}
+      command_handler (command);
-
-      make_command_stats_cleanup (1);
-
-      /* Do not execute commented lines.  */
-      if (command[0] != '#')
-	{
-	  execute_command (command, instream == stdin);
-
-	  /* Do any commands attached to breakpoint we are stopped at.  */
-	  bpstat_do_actions ();
-	}
-      do_cleanups (old_chain);
     }
 }
 
@@ -1016,32 +996,26 @@ gdb_safe_append_history (void)
   do_cleanups (old_chain);
 }
 
-/* Read one line from the command input stream `instream'
+/* Read one line from the command input stream `instream' into a local
-   into the local static buffer `linebuffer' (whose current length
+   static buffer.  The buffer is made bigger as necessary.  Returns
-   is `linelength').
+   the address of the start of the line.
-   The buffer is made bigger as necessary.
-   Returns the address of the start of the line.
 
    NULL is returned for end of file.
 
-   *If* the instream == stdin & stdin is a terminal, the line read
+   *If* the instream == stdin & stdin is a terminal, the line read is
-   is copied into the file line saver (global var char *line,
+   copied into the global 'saved_command_line' so that it can be
-   length linesize) so that it can be duplicated.
+   repeated.
 
-   This routine either uses fancy command line editing or
+   This routine either uses fancy command line editing or simple input
-   simple input as the user has requested.  */
+   as the user has requested.  */
 
 char *
 command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
 {
-  static char *linebuffer = 0;
+  static struct buffer cmd_line_buffer;
-  static unsigned linelength = 0;
+  static int cmd_line_buffer_initialized;
   const char *prompt = prompt_arg;
-  char *p;
+  char *cmd;
-  char *p1;
-  char *rl;
-  char *nline;
-  char got_eof = 0;
 
   /* The annotation suffix must be non-NULL.  */
   if (annotation_suffix == NULL)
@@ -1065,13 +1039,14 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
       prompt = local_prompt;
     }
 
-  if (linebuffer == 0)
+  if (!cmd_line_buffer_initialized)
     {
-      linelength = 80;
+      buffer_init (&cmd_line_buffer);
-      linebuffer = (char *) xmalloc (linelength);
+      cmd_line_buffer_initialized = 1;
     }
 
-  p = linebuffer;
+  /* Starting a new command line.  */
+  cmd_line_buffer.used_size = 0;
 
   /* Control-C quits instantly if typed while in this loop
      since it should not wait until the user types a newline.  */
@@ -1084,6 +1059,8 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
 
   while (1)
     {
+      char *rl;
+
       /* 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.  */
@@ -1115,37 +1092,16 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
 	  rl = gdb_readline_no_editing (prompt);
 	}
 
-      if (annotation_level > 1 && instream == stdin)
+      cmd = handle_line_of_input (&cmd_line_buffer, rl,
+				  repeat, annotation_suffix);
+      if (cmd == (char *) EOF)
 	{
-	  puts_unfiltered ("\n\032\032post-");
+	  cmd = NULL;
-	  puts_unfiltered (annotation_suffix);
-	  puts_unfiltered ("\n");
-	}
-
-      if (!rl || rl == (char *) EOF)
-	{
-	  got_eof = 1;
 	  break;
 	}
-      if (strlen (rl) + 1 + (p - linebuffer) > linelength)
+      if (cmd != NULL)
-	{
-	  linelength = strlen (rl) + 1 + (p - linebuffer);
-	  nline = (char *) xrealloc (linebuffer, linelength);
-	  p += nline - linebuffer;
-	  linebuffer = nline;
-	}
-      p1 = rl;
-      /* Copy line.  Don't copy null at end.  (Leaves line alone
-         if this was just a newline).  */
-      while (*p1)
-	*p++ = *p1++;
-
-      xfree (rl);		/* Allocated in readline.  */
-
-      if (p == linebuffer || *(p - 1) != '\\')
 	break;
 
-      p--;			/* Put on top of '\'.  */
       prompt = NULL;
     }
 
@@ -1155,77 +1111,7 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
 #endif
   immediate_quit--;
 
-  if (got_eof)
+  return cmd;
-    return NULL;
-
-#define SERVER_COMMAND_LENGTH 7
-  server_command =
-    (p - linebuffer > SERVER_COMMAND_LENGTH)
-    && strncmp (linebuffer, "server ", SERVER_COMMAND_LENGTH) == 0;
-  if (server_command)
-    {
-      /* Note that we don't set `line'.  Between this and the check in
-         dont_repeat, this insures that repeating will still do the
-         right thing.  */
-      *p = '\0';
-      return linebuffer + SERVER_COMMAND_LENGTH;
-    }
-
-  /* Do history expansion if that is wished.  */
-  if (history_expansion_p && instream == stdin
-      && ISATTY (instream))
-    {
-      char *history_value;
-      int expanded;
-
-      *p = '\0';		/* Insert null now.  */
-      expanded = history_expand (linebuffer, &history_value);
-      if (expanded)
-	{
-	  /* Print the changes.  */
-	  printf_unfiltered ("%s\n", history_value);
-
-	  /* If there was an error, call this function again.  */
-	  if (expanded < 0)
-	    {
-	      xfree (history_value);
-	      return command_line_input (prompt, repeat,
-					 annotation_suffix);
-	    }
-	  if (strlen (history_value) > linelength)
-	    {
-	      linelength = strlen (history_value) + 1;
-	      linebuffer = (char *) xrealloc (linebuffer, linelength);
-	    }
-	  strcpy (linebuffer, history_value);
-	  p = linebuffer + strlen (linebuffer);
-	}
-      xfree (history_value);
-    }
-
-  /* If we just got an empty line, and that is supposed to repeat the
-     previous command, return the value in the global buffer.  */
-  if (repeat && p == linebuffer)
-    return saved_command_line;
-  for (p1 = linebuffer; *p1 == ' ' || *p1 == '\t'; p1++);
-  if (repeat && !*p1)
-    return saved_command_line;
-
-  *p = 0;
-
-  /* Add line to history if appropriate.  */
-  if (*linebuffer && input_from_terminal_p ())
-    gdb_add_history (linebuffer);
-
-  /* Save into global buffer if appropriate.  */
-  if (repeat)
-    {
-      xfree (saved_command_line);
-      saved_command_line = xstrdup (linebuffer);
-      return saved_command_line;
-    }
-
-  return linebuffer;
 }
 
 /* Print the GDB banner.  */

gdb/top.h

@@ -20,6 +20,8 @@
 #ifndef TOP_H
 #define TOP_H
 
+struct buffer;
+
 /* From top.c.  */
 extern char *saved_command_line;
 extern FILE *instream;
@@ -97,4 +99,8 @@ extern void set_verbose (char *, int, struct cmd_list_element *);
 
 extern void do_restore_instream_cleanup (void *stream);
 
+extern char *handle_line_of_input (struct buffer *cmd_line_buffer,
+				   char *rl, int repeat,
+				   char *annotation_suffix);
+
 #endif