[PATCH 00/10] Command line input handling TLC

[PATCH 00/10] Command line input handling TLC

[Pedro Alves]

I looked a bit at changing immediate_quit users in order to get rid of
throws from signal handlers, for the C++ conversion, and ended up
stumbling on gdb's input code...

So today I split this patch:

  [PATCH 02/23] Command line input handling TLC
  https://sourceware.org/ml/gdb-patches/2016-02/msg00070.html

into smaller pieces.  I think this can go in independently of the rest
of that series.

The original motivation here was factor out all the globals used by
the command line input handling code to a structure, so that we can
keep multiple instances of that later on.  But, I found that this code
is a lot more messier than it needs to be, thus this series cleans
things up significantly in the process as well.

I tried to split the last patch further into smaller independent
pieces, but failed...

Pedro Alves (10):
  Test issuing a command split in multiple lines with continuation chars
  Garbage collect window_hook
  gdb_readline2 -> gdb_readline_callback_no_editing
  Eliminate async_annotation_suffix
  Update prompt_for_continue comments
  gdb_readline -> gdb_readline_no_editing
  Use struct buffer in gdb_readline_no_editing
  Use struct buffer in gdb_readline_callback_no_editing
  Simplify saved_command_line handling
  Command line input handling TLC

 gdb/common/buffer.h                           |   8 +
 gdb/defs.h                                    |   2 -
 gdb/event-top.c                               | 456 ++++++++++++--------------
 gdb/event-top.h                               |   5 +-
 gdb/main.c                                    |   3 +-
 gdb/mi/mi-interp.c                            |   2 +-
 gdb/testsuite/gdb.base/command-line-input.exp |  36 ++
 gdb/top.c                                     | 239 +++-----------
 gdb/top.h                                     |   7 +-
 gdb/utils.c                                   |  22 +-
 10 files changed, 318 insertions(+), 462 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/command-line-input.exp

-- 
1.9.3

[PATCH 07/10] Use struct buffer in gdb_readline_no_editing

[Pedro Alves]

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* common/buffer.h (buffer_grow_char): New function.
	* top.c: Include buffer.h.
	(gdb_readline_no_editing): Use struct buffer instead of xrealloc.
---
 gdb/common/buffer.h |  8 ++++++++
 gdb/top.c           | 40 ++++++++++++++++++----------------------
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/gdb/common/buffer.h b/gdb/common/buffer.h
index 2a47db4..bad2c3a 100644
--- a/gdb/common/buffer.h
+++ b/gdb/common/buffer.h
@@ -31,6 +31,14 @@ struct buffer
    accommodate the new data.  */
 void buffer_grow (struct buffer *buffer, const char *data, size_t size);
 
+/* Append CH to the end of BUFFER.  Grows the buffer to accommodate
+   the new data.  */
+static inline void
+buffer_grow_char (struct buffer *buffer, char c)
+{
+  buffer_grow (buffer, &c, 1);
+}
+
 /* Release any memory held by BUFFER.  */
 void buffer_free (struct buffer *buffer);
 
diff --git a/gdb/top.c b/gdb/top.c
index e781cdd..558f943 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -50,6 +50,7 @@
 #include "maint.h"
 #include "filenames.h"
 #include "frame.h"
+#include "buffer.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -603,65 +604,60 @@ prevent_dont_repeat (void)
 \f
 /* Read a line from the stream "instream" without command line editing.
 
-   It prints PROMPT_ARG once at the start.
+   It prints PROMPT once at the start.
    Action is compatible with "readline", e.g. space for the result is
    malloc'd and should be freed by the caller.
 
    A NULL return means end of file.  */
 
 static char *
-gdb_readline_no_editing (const char *prompt_arg)
+gdb_readline_no_editing (const char *prompt)
 {
-  int c;
-  char *result;
-  int input_index = 0;
-  int result_size = 80;
+  struct buffer line_buffer;
 
-  if (prompt_arg)
+  buffer_init (&line_buffer);
+
+  if (prompt != NULL)
     {
       /* Don't use a _filtered function here.  It causes the assumed
          character position to be off, since the newline we read from
          the user is not accounted for.  */
-      fputs_unfiltered (prompt_arg, gdb_stdout);
+      fputs_unfiltered (prompt, gdb_stdout);
       gdb_flush (gdb_stdout);
     }
 
-  result = (char *) xmalloc (result_size);
-
   while (1)
     {
+      int c;
+
       /* Read from stdin if we are executing a user defined command.
          This is the right thing for prompt_for_continue, at least.  */
       c = fgetc (instream ? instream : stdin);
 
       if (c == EOF)
 	{
-	  if (input_index > 0)
+	  if (line_buffer.used_size > 0)
 	    /* The last line does not end with a newline.  Return it, and
 	       if we are called again fgetc will still return EOF and
 	       we'll return NULL then.  */
 	    break;
-	  xfree (result);
+	  xfree (buffer_finish (&line_buffer));
 	  return NULL;
 	}
 
       if (c == '\n')
 	{
-	  if (input_index > 0 && result[input_index - 1] == '\r')
-	    input_index--;
+	  if (line_buffer.used_size > 0
+	      && line_buffer.buffer[line_buffer.used_size - 1] == '\r')
+	    line_buffer.used_size--;
 	  break;
 	}
 
-      result[input_index++] = c;
-      while (input_index >= result_size)
-	{
-	  result_size *= 2;
-	  result = (char *) xrealloc (result, result_size);
-	}
+      buffer_grow_char (&line_buffer, c);
     }
 
-  result[input_index++] = '\0';
-  return result;
+  buffer_grow_char (&line_buffer, '\0');
+  return buffer_finish (&line_buffer);
 }
 
 /* Variables which control command line editing and history
-- 
1.9.3

Re: [PATCH 07/10] Use struct buffer in gdb_readline_no_editing

[Sergio Durigan Junior]

On Thursday, February 18 2016, Pedro Alves wrote:

> gdb/ChangeLog:
> 2016-02-18  Pedro Alves  <palves@redhat.com>
>
> 	* common/buffer.h (buffer_grow_char): New function.
> 	* top.c: Include buffer.h.
> 	(gdb_readline_no_editing): Use struct buffer instead of xrealloc.

I think you also have to mention here that you renamed PROMPT_ARG to
PROMPT on gdb_readline_no_editing.

> ---
>  gdb/common/buffer.h |  8 ++++++++
>  gdb/top.c           | 40 ++++++++++++++++++----------------------
>  2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/gdb/common/buffer.h b/gdb/common/buffer.h
> index 2a47db4..bad2c3a 100644
> --- a/gdb/common/buffer.h
> +++ b/gdb/common/buffer.h
> @@ -31,6 +31,14 @@ struct buffer
>     accommodate the new data.  */
>  void buffer_grow (struct buffer *buffer, const char *data, size_t size);
>  
> +/* Append CH to the end of BUFFER.  Grows the buffer to accommodate
> +   the new data.  */
> +static inline void
> +buffer_grow_char (struct buffer *buffer, char c)
> +{
> +  buffer_grow (buffer, &c, 1);
> +}

Missing newline between comment and function.

> +
>  /* Release any memory held by BUFFER.  */
>  void buffer_free (struct buffer *buffer);
>  
> diff --git a/gdb/top.c b/gdb/top.c
> index e781cdd..558f943 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -50,6 +50,7 @@
>  #include "maint.h"
>  #include "filenames.h"
>  #include "frame.h"
> +#include "buffer.h"
>  
>  /* readline include files.  */
>  #include "readline/readline.h"
> @@ -603,65 +604,60 @@ prevent_dont_repeat (void)
>  \f
>  /* Read a line from the stream "instream" without command line editing.
>  
> -   It prints PROMPT_ARG once at the start.
> +   It prints PROMPT once at the start.
>     Action is compatible with "readline", e.g. space for the result is
>     malloc'd and should be freed by the caller.
>  
>     A NULL return means end of file.  */
>  
>  static char *
> -gdb_readline_no_editing (const char *prompt_arg)
> +gdb_readline_no_editing (const char *prompt)
>  {
> -  int c;
> -  char *result;
> -  int input_index = 0;
> -  int result_size = 80;
> +  struct buffer line_buffer;
>  
> -  if (prompt_arg)
> +  buffer_init (&line_buffer);
> +
> +  if (prompt != NULL)
>      {
>        /* Don't use a _filtered function here.  It causes the assumed
>           character position to be off, since the newline we read from
>           the user is not accounted for.  */
> -      fputs_unfiltered (prompt_arg, gdb_stdout);
> +      fputs_unfiltered (prompt, gdb_stdout);
>        gdb_flush (gdb_stdout);
>      }
>  
> -  result = (char *) xmalloc (result_size);
> -
>    while (1)
>      {
> +      int c;
> +
>        /* Read from stdin if we are executing a user defined command.
>           This is the right thing for prompt_for_continue, at least.  */
>        c = fgetc (instream ? instream : stdin);
>  
>        if (c == EOF)
>  	{
> -	  if (input_index > 0)
> +	  if (line_buffer.used_size > 0)
>  	    /* The last line does not end with a newline.  Return it, and
>  	       if we are called again fgetc will still return EOF and
>  	       we'll return NULL then.  */
>  	    break;
> -	  xfree (result);
> +	  xfree (buffer_finish (&line_buffer));
>  	  return NULL;
>  	}
>  
>        if (c == '\n')
>  	{
> -	  if (input_index > 0 && result[input_index - 1] == '\r')
> -	    input_index--;
> +	  if (line_buffer.used_size > 0
> +	      && line_buffer.buffer[line_buffer.used_size - 1] == '\r')
> +	    line_buffer.used_size--;
>  	  break;
>  	}
>  
> -      result[input_index++] = c;
> -      while (input_index >= result_size)
> -	{
> -	  result_size *= 2;
> -	  result = (char *) xrealloc (result, result_size);
> -	}
> +      buffer_grow_char (&line_buffer, c);
>      }
>  
> -  result[input_index++] = '\0';
> -  return result;
> +  buffer_grow_char (&line_buffer, '\0');
> +  return buffer_finish (&line_buffer);
>  }
>  
>  /* Variables which control command line editing and history
> -- 
> 1.9.3

LGTM after addressing the comments.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 07/10] Use struct buffer in gdb_readline_no_editing

[Pedro Alves]

On 02/18/2016 10:16 PM, Sergio Durigan Junior wrote:
> On Thursday, February 18 2016, Pedro Alves wrote:
>
>> gdb/ChangeLog:
>> 2016-02-18  Pedro Alves  <palves@redhat.com>
>>
>> 	* common/buffer.h (buffer_grow_char): New function.
>> 	* top.c: Include buffer.h.
>> 	(gdb_readline_no_editing): Use struct buffer instead of xrealloc.
>
> I think you also have to mention here that you renamed PROMPT_ARG to
> PROMPT on gdb_readline_no_editing.

Indeed.  Done.

>> ---
>>   gdb/common/buffer.h |  8 ++++++++
>>   gdb/top.c           | 40 ++++++++++++++++++----------------------
>>   2 files changed, 26 insertions(+), 22 deletions(-)
>>
>> diff --git a/gdb/common/buffer.h b/gdb/common/buffer.h
>> index 2a47db4..bad2c3a 100644
>> --- a/gdb/common/buffer.h
>> +++ b/gdb/common/buffer.h
>> @@ -31,6 +31,14 @@ struct buffer
>>      accommodate the new data.  */
>>   void buffer_grow (struct buffer *buffer, const char *data, size_t size);
>>
>> +/* Append CH to the end of BUFFER.  Grows the buffer to accommodate
>> +   the new data.  */
>> +static inline void
>> +buffer_grow_char (struct buffer *buffer, char c)
>> +{
>> +  buffer_grow (buffer, &c, 1);
>> +}
>
> Missing newline between comment and function.
>

Yeah, this is a borderline case, since it is both a
declaration and a definition.  I've added the newline.

> LGTM after addressing the comments.

Thanks.  Below's what I pushed.

 From 7a3bde34bc61af108556c74b661533dadddcb178 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Mar 2016 18:25:00 +0000
Subject: [PATCH 07/10] Use struct buffer in gdb_readline_no_editing

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

	* common/buffer.h (buffer_grow_char): New function.
	* top.c: Include buffer.h.
	(gdb_readline_no_editing): Rename 'prompt_arg' parameter to
	'prompt'.  Use struct buffer instead of xrealloc.
---
  gdb/ChangeLog       |  7 +++++++
  gdb/common/buffer.h |  9 +++++++++
  gdb/top.c           | 40 ++++++++++++++++++----------------------
  3 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 06d1373..bea8172 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
  2016-03-09  Pedro Alves  <palves@redhat.com>

+	* common/buffer.h (buffer_grow_char): New function.
+	* top.c: Include buffer.h.
+	(gdb_readline_no_editing): Rename 'prompt_arg' parameter to
+	'prompt'.  Use struct buffer instead of xrealloc.
+
+2016-03-09  Pedro Alves  <palves@redhat.com>
+
  	* defs.h (gdb_readline): Delete declaration.
  	* top.c (gdb_readline): Rename to ...
  	(gdb_readline_no_editing): ... this, and make static.
diff --git a/gdb/common/buffer.h b/gdb/common/buffer.h
index 2a47db4..8122a2c 100644
--- a/gdb/common/buffer.h
+++ b/gdb/common/buffer.h
@@ -31,6 +31,15 @@ struct buffer
     accommodate the new data.  */
  void buffer_grow (struct buffer *buffer, const char *data, size_t size);

+/* Append C to the end of BUFFER.  Grows the buffer to accommodate the
+   new data.  */
+
+static inline void
+buffer_grow_char (struct buffer *buffer, char c)
+{
+  buffer_grow (buffer, &c, 1);
+}
+
  /* Release any memory held by BUFFER.  */
  void buffer_free (struct buffer *buffer);

diff --git a/gdb/top.c b/gdb/top.c
index e781cdd..558f943 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -50,6 +50,7 @@
  #include "maint.h"
  #include "filenames.h"
  #include "frame.h"
+#include "buffer.h"

  /* readline include files.  */
  #include "readline/readline.h"
@@ -603,65 +604,60 @@ prevent_dont_repeat (void)
  \f
  /* Read a line from the stream "instream" without command line editing.

-   It prints PROMPT_ARG once at the start.
+   It prints PROMPT once at the start.
     Action is compatible with "readline", e.g. space for the result is
     malloc'd and should be freed by the caller.

     A NULL return means end of file.  */

  static char *
-gdb_readline_no_editing (const char *prompt_arg)
+gdb_readline_no_editing (const char *prompt)
  {
-  int c;
-  char *result;
-  int input_index = 0;
-  int result_size = 80;
+  struct buffer line_buffer;

-  if (prompt_arg)
+  buffer_init (&line_buffer);
+
+  if (prompt != NULL)
      {
        /* Don't use a _filtered function here.  It causes the assumed
           character position to be off, since the newline we read from
           the user is not accounted for.  */
-      fputs_unfiltered (prompt_arg, gdb_stdout);
+      fputs_unfiltered (prompt, gdb_stdout);
        gdb_flush (gdb_stdout);
      }

-  result = (char *) xmalloc (result_size);
-
    while (1)
      {
+      int c;
+
        /* Read from stdin if we are executing a user defined command.
           This is the right thing for prompt_for_continue, at least.  */
        c = fgetc (instream ? instream : stdin);

        if (c == EOF)
  	{
-	  if (input_index > 0)
+	  if (line_buffer.used_size > 0)
  	    /* The last line does not end with a newline.  Return it, and
  	       if we are called again fgetc will still return EOF and
  	       we'll return NULL then.  */
  	    break;
-	  xfree (result);
+	  xfree (buffer_finish (&line_buffer));
  	  return NULL;
  	}

        if (c == '\n')
  	{
-	  if (input_index > 0 && result[input_index - 1] == '\r')
-	    input_index--;
+	  if (line_buffer.used_size > 0
+	      && line_buffer.buffer[line_buffer.used_size - 1] == '\r')
+	    line_buffer.used_size--;
  	  break;
  	}

-      result[input_index++] = c;
-      while (input_index >= result_size)
-	{
-	  result_size *= 2;
-	  result = (char *) xrealloc (result, result_size);
-	}
+      buffer_grow_char (&line_buffer, c);
      }

-  result[input_index++] = '\0';
-  return result;
+  buffer_grow_char (&line_buffer, '\0');
+  return buffer_finish (&line_buffer);
  }

  /* Variables which control command line editing and history
-- 
2.5.0

Re: [PATCH 07/10] Use struct buffer in gdb_readline_no_editing

[Simon Marchi]

On 16-02-18 12:40 PM, Pedro Alves wrote:
> gdb/ChangeLog:
> 2016-02-18  Pedro Alves  <palves@redhat.com>
> 
> 	* common/buffer.h (buffer_grow_char): New function.
> 	* top.c: Include buffer.h.
> 	(gdb_readline_no_editing): Use struct buffer instead of xrealloc.
> ---
>  gdb/common/buffer.h |  8 ++++++++
>  gdb/top.c           | 40 ++++++++++++++++++----------------------
>  2 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/gdb/common/buffer.h b/gdb/common/buffer.h
> index 2a47db4..bad2c3a 100644
> --- a/gdb/common/buffer.h
> +++ b/gdb/common/buffer.h
> @@ -31,6 +31,14 @@ struct buffer
>     accommodate the new data.  */
>  void buffer_grow (struct buffer *buffer, const char *data, size_t size);
>  
> +/* Append CH to the end of BUFFER.  Grows the buffer to accommodate

Tiny-micro-nit: the actual parameter name is c.

Re: [PATCH 07/10] Use struct buffer in gdb_readline_no_editing

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> +/* Append CH to the end of BUFFER.  Grows the buffer to accommodate
> +   the new data.  */
> +static inline void
> +buffer_grow_char (struct buffer *buffer, char c)

Nit: s/Append CH /Append C /

-- 
Yao (齐尧)

[PATCH 01/10] Test issuing a command split in multiple lines with continuation chars

[Pedro Alves]

I happened to break this locally and the testsuite didn't notice it.
Add some tests.

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* gdb.base/command-line-input.exp: New file.
---
 gdb/testsuite/gdb.base/command-line-input.exp | 36 +++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/command-line-input.exp

diff --git a/gdb/testsuite/gdb.base/command-line-input.exp b/gdb/testsuite/gdb.base/command-line-input.exp
new file mode 100644
index 0000000..0a9d44b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/command-line-input.exp
@@ -0,0 +1,36 @@
+# Copyright (C) 2014-2016 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Command line input testing.
+
+# Test issuing a command split in multiple lines with continuation
+# characters.
+
+gdb_exit
+gdb_start
+
+set test "print 1\\\\n + 2"
+gdb_test_multiple "print 1\\\n + 2" $test {
+    -re "^print 1\\\\\r\n \\+ 2\r\n\\\$$decimal = 3\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
+set test "print 1\\\\n2"
+gdb_test_multiple "print 1\\\n2" $test {
+    -re "^print 1\\\\\r\n2\r\n\\\$$decimal = 12\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
-- 
1.9.3

Re: [PATCH 01/10] Test issuing a command split in multiple lines with continuation chars

[Sergio Durigan Junior]

On Thursday, February 18 2016, Pedro Alves wrote:

> I happened to break this locally and the testsuite didn't notice it.
> Add some tests.
>
> gdb/ChangeLog:
> 2016-02-18  Pedro Alves  <palves@redhat.com>
>
> 	* gdb.base/command-line-input.exp: New file.
> ---
>  gdb/testsuite/gdb.base/command-line-input.exp | 36 +++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/command-line-input.exp
>
> diff --git a/gdb/testsuite/gdb.base/command-line-input.exp b/gdb/testsuite/gdb.base/command-line-input.exp
> new file mode 100644
> index 0000000..0a9d44b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/command-line-input.exp
> @@ -0,0 +1,36 @@
> +# Copyright (C) 2014-2016 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Command line input testing.
> +
> +# Test issuing a command split in multiple lines with continuation
> +# characters.
> +
> +gdb_exit
> +gdb_start
> +
> +set test "print 1\\\\n + 2"

I was worried that this would insert a newline in gdb.sum, but it
doesn't.

> +gdb_test_multiple "print 1\\\n + 2" $test {
> +    -re "^print 1\\\\\r\n \\+ 2\r\n\\\$$decimal = 3\r\n$gdb_prompt $" {
> +	pass $test
> +    }
> +}
> +
> +set test "print 1\\\\n2"
> +gdb_test_multiple "print 1\\\n2" $test {
> +    -re "^print 1\\\\\r\n2\r\n\\\$$decimal = 12\r\n$gdb_prompt $" {
> +	pass $test
> +    }
> +}
> -- 
> 1.9.3

This looks good to me.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 01/10] Test issuing a command split in multiple lines with continuation chars

[Luis Machado]

On 02/18/2016 03:40 PM, Pedro Alves wrote:
> I happened to break this locally and the testsuite didn't notice it.
> Add some tests.
>
> gdb/ChangeLog:
> 2016-02-18  Pedro Alves  <palves@redhat.com>
>
> 	* gdb.base/command-line-input.exp: New file.
> ---
>   gdb/testsuite/gdb.base/command-line-input.exp | 36 +++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.base/command-line-input.exp
>
> diff --git a/gdb/testsuite/gdb.base/command-line-input.exp b/gdb/testsuite/gdb.base/command-line-input.exp
> new file mode 100644
> index 0000000..0a9d44b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/command-line-input.exp
> @@ -0,0 +1,36 @@
> +# Copyright (C) 2014-2016 Free Software Foundation, Inc.
> +#

Should this be 2016, since it is a new file?

Re: [PATCH 01/10] Test issuing a command split in multiple lines with continuation chars

[Pedro Alves]

On 02/24/2016 12:40 PM, Luis Machado wrote:
> On 02/18/2016 03:40 PM, Pedro Alves wrote:
>> I happened to break this locally and the testsuite didn't notice it.
>> Add some tests.
>>
>> gdb/ChangeLog:
>> 2016-02-18  Pedro Alves  <palves@redhat.com>
>>
>>     * gdb.base/command-line-input.exp: New file.
>> ---
>>   gdb/testsuite/gdb.base/command-line-input.exp | 36
>> +++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>   create mode 100644 gdb/testsuite/gdb.base/command-line-input.exp
>>
>> diff --git a/gdb/testsuite/gdb.base/command-line-input.exp
>> b/gdb/testsuite/gdb.base/command-line-input.exp
>> new file mode 100644
>> index 0000000..0a9d44b
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/command-line-input.exp
>> @@ -0,0 +1,36 @@
>> +# Copyright (C) 2014-2016 Free Software Foundation, Inc.
>> +#
>
> Should this be 2016, since it is a new file?

Whoops, indeed.  Fixed and pushed.

Thanks,
Pedro Alves

[PATCH 02/10] Garbage collect window_hook

[Pedro Alves]

I checked, and Insight doesn't set this.

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* top.c (window_hook): Delete.
	(command_loop): Remove references to window_hook.
---
 gdb/top.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/gdb/top.c b/gdb/top.c
index ed200ba..b5ee4af 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -124,12 +124,6 @@ char *current_directory;
 /* The directory name is actually stored here (usually).  */
 char gdb_dirbuf[1024];
 
-/* Function to call before reading a command, if nonzero.
-   The function receives two args: an input stream,
-   and a prompt string.  */
-
-void (*window_hook) (FILE *, char *);
-
 /* Buffer used for reading command lines, and the size
    allocated for it so far.  */
 
@@ -545,9 +539,6 @@ command_loop (void)
 
   while (instream && !feof (instream))
     {
-      if (window_hook && instream == stdin)
-	(*window_hook) (instream, get_prompt ());
-
       clear_quit_flag ();
       if (instream == stdin)
 	reinitialize_more_filter ();
-- 
1.9.3

Re: [PATCH 02/10] Garbage collect window_hook

[Sergio Durigan Junior]

On Thursday, February 18 2016, Pedro Alves wrote:

> I checked, and Insight doesn't set this.
>
> gdb/ChangeLog:
> 2016-02-18  Pedro Alves  <palves@redhat.com>
>
> 	* top.c (window_hook): Delete.
> 	(command_loop): Remove references to window_hook.
> ---
>  gdb/top.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/gdb/top.c b/gdb/top.c
> index ed200ba..b5ee4af 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -124,12 +124,6 @@ char *current_directory;
>  /* The directory name is actually stored here (usually).  */
>  char gdb_dirbuf[1024];
>  
> -/* Function to call before reading a command, if nonzero.
> -   The function receives two args: an input stream,
> -   and a prompt string.  */
> -
> -void (*window_hook) (FILE *, char *);
> -
>  /* Buffer used for reading command lines, and the size
>     allocated for it so far.  */
>  
> @@ -545,9 +539,6 @@ command_loop (void)
>  
>    while (instream && !feof (instream))
>      {
> -      if (window_hook && instream == stdin)
> -	(*window_hook) (instream, get_prompt ());
> -
>        clear_quit_flag ();
>        if (instream == stdin)
>  	reinitialize_more_filter ();
> -- 
> 1.9.3

LGTM.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

[PATCH 08/10] Use struct buffer in gdb_readline_callback_no_editing

[Pedro Alves]

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* event-top.c: Include buffer.h.
	(gdb_readline_callback_no_editing): Use struct buffer instead of
	xrealloc.
---
 gdb/event-top.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index ee44197..3447fc8 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -37,6 +37,7 @@
 #include "gdbcmd.h"		/* for dont_repeat() */
 #include "annotate.h"
 #include "maint.h"
+#include "buffer.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -382,7 +383,7 @@ top_level_prompt (void)
   return xstrdup (prompt);
 }
 
-/* When there is an event ready on the stdin file desriptor, instead
+/* 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_callback_no_editing, give gdb a
    chance to detect errors and do something.  */
@@ -678,10 +679,11 @@ gdb_readline_callback_no_editing (gdb_client_data client_data)
 {
   int c;
   char *result;
-  int input_index = 0;
-  int result_size = 80;
+  struct buffer line_buffer;
   static int done_once = 0;
 
+  buffer_init (&line_buffer);
+
   /* Unbuffer the input stream, so that, later on, the calls to fgetc
      fetch only one char at the time from the stream.  The fgetc's will
      get up to the first newline, but there may be more chars in the
@@ -694,8 +696,6 @@ gdb_readline_callback_no_editing (gdb_client_data client_data)
       done_once = 1;
     }
 
-  result = (char *) xmalloc (result_size);
-
   /* We still need the while loop here, even though it would seem
      obvious to invoke gdb_readline_callback_no_editing at every
      character entered.  If not using the readline library, the
@@ -712,32 +712,31 @@ gdb_readline_callback_no_editing (gdb_client_data client_data)
 
       if (c == EOF)
 	{
-	  if (input_index > 0)
-	    /* The last line does not end with a newline.  Return it,
-	       and if we are called again fgetc will still return EOF
-	       and we'll return NULL then.  */
-	    break;
-	  xfree (result);
+	  if (line_buffer.used_size > 0)
+	    {
+	      /* The last line does not end with a newline.  Return it, and
+		 if we are called again fgetc will still return EOF and
+		 we'll return NULL then.  */
+	      break;
+	    }
+	  xfree (buffer_finish (&line_buffer));
 	  (*input_handler) (0);
 	  return;
 	}
 
       if (c == '\n')
 	{
-	  if (input_index > 0 && result[input_index - 1] == '\r')
-	    input_index--;
+	  if (line_buffer.used_size > 0
+	      && line_buffer.buffer[line_buffer.used_size - 1] == '\r')
+	    line_buffer.used_size--;
 	  break;
 	}
 
-      result[input_index++] = c;
-      while (input_index >= result_size)
-	{
-	  result_size *= 2;
-	  result = (char *) xrealloc (result, result_size);
-	}
+      buffer_grow_char (&line_buffer, c);
     }
 
-  result[input_index++] = '\0';
+  buffer_grow_char (&line_buffer, '\0');
+  result = buffer_finish (&line_buffer);
   (*input_handler) (result);
 }
 \f
-- 
1.9.3

Re: [PATCH 08/10] Use struct buffer in gdb_readline_callback_no_editing

[Sergio Durigan Junior]

On Thursday, February 18 2016, Pedro Alves wrote:

> gdb/ChangeLog:
> 2016-02-18  Pedro Alves  <palves@redhat.com>
>
> 	* event-top.c: Include buffer.h.
> 	(gdb_readline_callback_no_editing): Use struct buffer instead of
> 	xrealloc.
> ---
>  gdb/event-top.c | 39 +++++++++++++++++++--------------------
>  1 file changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index ee44197..3447fc8 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -37,6 +37,7 @@
>  #include "gdbcmd.h"		/* for dont_repeat() */
>  #include "annotate.h"
>  #include "maint.h"
> +#include "buffer.h"
>  
>  /* readline include files.  */
>  #include "readline/readline.h"
> @@ -382,7 +383,7 @@ top_level_prompt (void)
>    return xstrdup (prompt);
>  }
>  
> -/* When there is an event ready on the stdin file desriptor, instead
> +/* 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_callback_no_editing, give gdb a
>     chance to detect errors and do something.  */
> @@ -678,10 +679,11 @@ gdb_readline_callback_no_editing (gdb_client_data client_data)
>  {
>    int c;
>    char *result;
> -  int input_index = 0;
> -  int result_size = 80;
> +  struct buffer line_buffer;
>    static int done_once = 0;
>  
> +  buffer_init (&line_buffer);
> +
>    /* Unbuffer the input stream, so that, later on, the calls to fgetc
>       fetch only one char at the time from the stream.  The fgetc's will
>       get up to the first newline, but there may be more chars in the
> @@ -694,8 +696,6 @@ gdb_readline_callback_no_editing (gdb_client_data client_data)
>        done_once = 1;
>      }
>  
> -  result = (char *) xmalloc (result_size);
> -
>    /* We still need the while loop here, even though it would seem
>       obvious to invoke gdb_readline_callback_no_editing at every
>       character entered.  If not using the readline library, the
> @@ -712,32 +712,31 @@ gdb_readline_callback_no_editing (gdb_client_data client_data)
>  
>        if (c == EOF)
>  	{
> -	  if (input_index > 0)
> -	    /* The last line does not end with a newline.  Return it,
> -	       and if we are called again fgetc will still return EOF
> -	       and we'll return NULL then.  */
> -	    break;
> -	  xfree (result);
> +	  if (line_buffer.used_size > 0)
> +	    {
> +	      /* The last line does not end with a newline.  Return it, and
> +		 if we are called again fgetc will still return EOF and
> +		 we'll return NULL then.  */
> +	      break;
> +	    }
> +	  xfree (buffer_finish (&line_buffer));
>  	  (*input_handler) (0);
>  	  return;
>  	}
>  
>        if (c == '\n')
>  	{
> -	  if (input_index > 0 && result[input_index - 1] == '\r')
> -	    input_index--;
> +	  if (line_buffer.used_size > 0
> +	      && line_buffer.buffer[line_buffer.used_size - 1] == '\r')
> +	    line_buffer.used_size--;
>  	  break;
>  	}
>  
> -      result[input_index++] = c;
> -      while (input_index >= result_size)
> -	{
> -	  result_size *= 2;
> -	  result = (char *) xrealloc (result, result_size);
> -	}
> +      buffer_grow_char (&line_buffer, c);
>      }
>  
> -  result[input_index++] = '\0';
> +  buffer_grow_char (&line_buffer, '\0');
> +  result = buffer_finish (&line_buffer);
>    (*input_handler) (result);
>  }
>  \f
> -- 
> 1.9.3

LGTM.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

[PATCH 06/10] gdb_readline -> gdb_readline_no_editing

[Pedro Alves]

Name this such that it's clearer that this is not a wrapper for the
real readline, but instead a replacement that provides no command line
editing features.

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* defs.h (gdb_readline): Delete declaration.
	* top.c (gdb_readline): Rename to ...
	(gdb_readline_no_editing): ... this, and make static.
---
 gdb/defs.h | 2 --
 gdb/top.c  | 7 ++++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/gdb/defs.h b/gdb/defs.h
index f6ffeac..b94df30 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -285,8 +285,6 @@ extern void print_transfer_performance (struct ui_file *stream,
 
 typedef void initialize_file_ftype (void);
 
-extern char *gdb_readline (const char *);
-
 extern char *gdb_readline_wrapper (const char *);
 
 extern char *command_line_input (const char *, int, char *);
diff --git a/gdb/top.c b/gdb/top.c
index fb1657a..e781cdd 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -608,8 +608,9 @@ prevent_dont_repeat (void)
    malloc'd and should be freed by the caller.
 
    A NULL return means end of file.  */
-char *
-gdb_readline (const char *prompt_arg)
+
+static char *
+gdb_readline_no_editing (const char *prompt_arg)
 {
   int c;
   char *result;
@@ -1117,7 +1118,7 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
 	}
       else
 	{
-	  rl = gdb_readline (prompt);
+	  rl = gdb_readline_no_editing (prompt);
 	}
 
       if (annotation_level > 1 && instream == stdin)
-- 
1.9.3

Re: [PATCH 06/10] gdb_readline -> gdb_readline_no_editing

[Sergio Durigan Junior]

On Thursday, February 18 2016, Pedro Alves wrote:

> Name this such that it's clearer that this is not a wrapper for the
> real readline, but instead a replacement that provides no command line
> editing features.
>
> gdb/ChangeLog:
> 2016-02-18  Pedro Alves  <palves@redhat.com>
>
> 	* defs.h (gdb_readline): Delete declaration.
> 	* top.c (gdb_readline): Rename to ...
> 	(gdb_readline_no_editing): ... this, and make static.
> ---
>  gdb/defs.h | 2 --
>  gdb/top.c  | 7 ++++---
>  2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/defs.h b/gdb/defs.h
> index f6ffeac..b94df30 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -285,8 +285,6 @@ extern void print_transfer_performance (struct ui_file *stream,
>  
>  typedef void initialize_file_ftype (void);
>  
> -extern char *gdb_readline (const char *);
> -
>  extern char *gdb_readline_wrapper (const char *);
>  
>  extern char *command_line_input (const char *, int, char *);
> diff --git a/gdb/top.c b/gdb/top.c
> index fb1657a..e781cdd 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -608,8 +608,9 @@ prevent_dont_repeat (void)
>     malloc'd and should be freed by the caller.
>  
>     A NULL return means end of file.  */
> -char *
> -gdb_readline (const char *prompt_arg)
> +
> +static char *
> +gdb_readline_no_editing (const char *prompt_arg)
>  {
>    int c;
>    char *result;
> @@ -1117,7 +1118,7 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
>  	}
>        else
>  	{
> -	  rl = gdb_readline (prompt);
> +	  rl = gdb_readline_no_editing (prompt);
>  	}
>  
>        if (annotation_level > 1 && instream == stdin)
> -- 
> 1.9.3

Thanks, I like this a lot.

LGTM.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

[PATCH 09/10] Simplify saved_command_line handling

[Pedro Alves]

There doesn't seem to be much point in trying to reuse this buffer.
Prefer simplicity instead.

(In case you're wondering whether this fixes an off-by-one: linelength
is misnamed; it's really a size including terminating null char.)

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* event-top.c (command_line_handler): Use xfree + xstrdup instead
	of xrealloc + strcpy.
	* main.c (captured_main): Use xstrdup instead of xmalloc plus
	manual clear.
	* top.c (saved_command_line): Rewrite comment.
	(saved_command_line_size): Delete.
	(command_line_input): Use xfree + xstrdup instead of xrealloc +
	strcpy.
	* top.h (saved_command_line_size): Delete declaration.
---
 gdb/event-top.c |  9 ++-------
 gdb/main.c      |  3 +--
 gdb/top.c       | 15 ++++-----------
 gdb/top.h       |  1 -
 4 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 3447fc8..221e242 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -650,13 +650,8 @@ command_line_handler (char *rl)
   /* Save into global buffer if appropriate.  */
   if (repeat)
     {
-      if (linelength > saved_command_line_size)
-	{
-	  saved_command_line
-	    = (char *) xrealloc (saved_command_line, linelength);
-	  saved_command_line_size = linelength;
-	}
-      strcpy (saved_command_line, linebuffer);
+      xfree (saved_command_line);
+      saved_command_line = xstrdup (linebuffer);
       if (!more_to_come)
 	{
 	  command_handler (saved_command_line);
diff --git a/gdb/main.c b/gdb/main.c
index a338b90..93ed98f 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -506,8 +506,7 @@ captured_main (void *data)
   ndir = 0;
 
   clear_quit_flag ();
-  saved_command_line = (char *) xmalloc (saved_command_line_size);
-  saved_command_line[0] = '\0';
+  saved_command_line = (char *) xstrdup ("");
   instream = stdin;
 
 #ifdef __MINGW32__
diff --git a/gdb/top.c b/gdb/top.c
index 558f943..1a5c3f9 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -125,11 +125,9 @@ char *current_directory;
 /* The directory name is actually stored here (usually).  */
 char gdb_dirbuf[1024];
 
-/* Buffer used for reading command lines, and the size
-   allocated for it so far.  */
-
+/* The last command line executed on the console.  Used for command
+   repetitions.  */
 char *saved_command_line;
-int saved_command_line_size = 100;
 
 /* Nonzero if the current command is modified by "server ".  This
    affects things like recording into the command history, commands
@@ -1222,13 +1220,8 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
   /* Save into global buffer if appropriate.  */
   if (repeat)
     {
-      if (linelength > saved_command_line_size)
-	{
-	  saved_command_line
-	    = (char *) xrealloc (saved_command_line, linelength);
-	  saved_command_line_size = linelength;
-	}
-      strcpy (saved_command_line, linebuffer);
+      xfree (saved_command_line);
+      saved_command_line = xstrdup (linebuffer);
       return saved_command_line;
     }
 
diff --git a/gdb/top.h b/gdb/top.h
index c450c6e..f3b080b 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -22,7 +22,6 @@
 
 /* From top.c.  */
 extern char *saved_command_line;
-extern int saved_command_line_size;
 extern FILE *instream;
 extern int in_user_command;
 extern int confirm;
-- 
1.9.3

[PATCH 03/10] gdb_readline2 -> gdb_readline_callback_no_editing

[Pedro Alves]

The "2" in "gdb_readline2" doesn't really convey much.  Rename for clarity.

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* event-top.c (gdb_readline2): Rename to ...
	(gdb_readline_callback_no_editing): ... this.
	(change_line_handler, stdin_event_handler)
	(gdb_setup_readline): Adjust.
	* event-top.h (gdb_readline2): Rename to ...
	(gdb_readline_callback_no_editing): ... this, and move closer to
	other readline-related declarations.
	* mi/mi-interp.c (mi_interpreter_resume): Adjust.
---
 gdb/event-top.c    | 36 ++++++++++++++++++------------------
 gdb/event-top.h    |  2 +-
 gdb/mi/mi-interp.c |  2 +-
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 2309cce..35830119 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -189,9 +189,9 @@ cli_command_loop (void *data)
 /* Change the function to be invoked every time there is a character
    ready on stdin.  This is used when the user sets the editing off,
    therefore bypassing readline, and letting gdb handle the input
-   itself, via gdb_readline2.  Also it is used in the opposite case in
-   which the user sets editing on again, by restoring readline
-   handling of the input.  */
+   itself, via gdb_readline_callback_no_editing.  Also it is used in
+   the opposite case in which the user sets editing on again, by
+   restoring readline handling of the input.  */
 static void
 change_line_handler (void)
 {
@@ -209,9 +209,9 @@ change_line_handler (void)
     }
   else
     {
-      /* Turn off editing by using gdb_readline2.  */
+      /* Turn off editing by using gdb_readline_callback_no_editing.  */
       gdb_rl_callback_handler_remove ();
-      call_readline = gdb_readline2;
+      call_readline = gdb_readline_callback_no_editing;
 
       /* Set up the command handler as well, in case we are called as
          first thing from .gdbinit.  */
@@ -410,8 +410,9 @@ top_level_prompt (void)
 
 /* When there is an event ready on the stdin file desriptor, instead
    of calling readline directly throught the callback function, or
-   instead of calling gdb_readline2, give gdb a chance to detect
-   errors and do something.  */
+   instead of calling gdb_readline_callback_no_editing, give gdb a
+   chance to detect errors and do something.  */
+
 void
 stdin_event_handler (int error, gdb_client_data client_data)
 {
@@ -699,13 +700,11 @@ command_line_handler (char *rl)
 }
 
 /* Does reading of input from terminal w/o the editing features
-   provided by the readline library.  */
+   provided by the readline library.  Calls the line input handler
+   once we have a whole input line.  */
 
-/* NOTE: 1999-04-30 Asynchronous version of gdb_readline; gdb_readline
-   will become obsolete when the event loop is made the default
-   execution for gdb.  */
 void
-gdb_readline2 (gdb_client_data client_data)
+gdb_readline_callback_no_editing (gdb_client_data client_data)
 {
   int c;
   char *result;
@@ -728,11 +727,12 @@ gdb_readline2 (gdb_client_data client_data)
   result = (char *) xmalloc (result_size);
 
   /* We still need the while loop here, even though it would seem
-     obvious to invoke gdb_readline2 at every character entered.  If
-     not using the readline library, the terminal is in cooked mode,
-     which sends the characters all at once.  Poll will notice that the
-     input fd has changed state only after enter is pressed.  At this
-     point we still need to fetch all the chars entered.  */
+     obvious to invoke gdb_readline_callback_no_editing at every
+     character entered.  If not using the readline library, the
+     terminal is in cooked mode, which sends the characters all at
+     once.  Poll will notice that the input fd has changed state only
+     after enter is pressed.  At this point we still need to fetch all
+     the chars entered.  */
 
   while (1)
     {
@@ -1055,7 +1055,7 @@ gdb_setup_readline (void)
   else
     {
       async_command_editing_p = 0;
-      call_readline = gdb_readline2;
+      call_readline = gdb_readline_callback_no_editing;
     }
   
   /* When readline has read an end-of-line character, it passes the
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 4f20770..64c6fdf 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -44,7 +44,6 @@ extern void handle_stop_sig (int sig);
 #endif
 extern void handle_sigint (int sig);
 extern void handle_sigterm (int sig);
-extern void gdb_readline2 (void *client_data);
 extern void async_request_quit (void *arg);
 extern void stdin_event_handler (int error, void *client_data);
 extern void async_disable_stdin (void);
@@ -62,6 +61,7 @@ extern void (*input_handler) (char *);
 extern int input_fd;
 extern void (*after_char_processing_hook) (void);
 extern int call_stdin_event_handler_again_p;
+extern void gdb_readline_callback_no_editing (void *client_data);
 
 /* Wrappers for rl_callback_handler_remove and
    rl_callback_handler_install that keep track of whether the callback
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 7f42367..80c8259 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -181,7 +181,7 @@ mi_interpreter_resume (void *data)
 
   /* These overwrite some of the initialization done in
      _intialize_event_loop.  */
-  call_readline = gdb_readline2;
+  call_readline = gdb_readline_callback_no_editing;
   input_handler = mi_execute_command_input_handler;
   async_command_editing_p = 0;
   /* FIXME: This is a total hack for now.  PB's use of the MI
-- 
1.9.3

Re: [PATCH 03/10] gdb_readline2 -> gdb_readline_callback_no_editing

[Sergio Durigan Junior]

On Thursday, February 18 2016, Pedro Alves wrote:

> The "2" in "gdb_readline2" doesn't really convey much.  Rename for clarity.

Any reason to put the "callback" in the middle of the name?  I usually
prefer to put it in the end.  "gdb_readline_no_editing_callback" seems
better to me.

Other than that, LGTM.

> gdb/ChangeLog:
> 2016-02-18  Pedro Alves  <palves@redhat.com>
>
> 	* event-top.c (gdb_readline2): Rename to ...
> 	(gdb_readline_callback_no_editing): ... this.
> 	(change_line_handler, stdin_event_handler)
> 	(gdb_setup_readline): Adjust.
> 	* event-top.h (gdb_readline2): Rename to ...
> 	(gdb_readline_callback_no_editing): ... this, and move closer to
> 	other readline-related declarations.
> 	* mi/mi-interp.c (mi_interpreter_resume): Adjust.
> ---
>  gdb/event-top.c    | 36 ++++++++++++++++++------------------
>  gdb/event-top.h    |  2 +-
>  gdb/mi/mi-interp.c |  2 +-
>  3 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index 2309cce..35830119 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -189,9 +189,9 @@ cli_command_loop (void *data)
>  /* Change the function to be invoked every time there is a character
>     ready on stdin.  This is used when the user sets the editing off,
>     therefore bypassing readline, and letting gdb handle the input
> -   itself, via gdb_readline2.  Also it is used in the opposite case in
> -   which the user sets editing on again, by restoring readline
> -   handling of the input.  */
> +   itself, via gdb_readline_callback_no_editing.  Also it is used in
> +   the opposite case in which the user sets editing on again, by
> +   restoring readline handling of the input.  */
>  static void
>  change_line_handler (void)
>  {
> @@ -209,9 +209,9 @@ change_line_handler (void)
>      }
>    else
>      {
> -      /* Turn off editing by using gdb_readline2.  */
> +      /* Turn off editing by using gdb_readline_callback_no_editing.  */
>        gdb_rl_callback_handler_remove ();
> -      call_readline = gdb_readline2;
> +      call_readline = gdb_readline_callback_no_editing;
>  
>        /* Set up the command handler as well, in case we are called as
>           first thing from .gdbinit.  */
> @@ -410,8 +410,9 @@ top_level_prompt (void)
>  
>  /* When there is an event ready on the stdin file desriptor, instead
>     of calling readline directly throught the callback function, or
> -   instead of calling gdb_readline2, give gdb a chance to detect
> -   errors and do something.  */
> +   instead of calling gdb_readline_callback_no_editing, give gdb a
> +   chance to detect errors and do something.  */
> +
>  void
>  stdin_event_handler (int error, gdb_client_data client_data)
>  {
> @@ -699,13 +700,11 @@ command_line_handler (char *rl)
>  }
>  
>  /* Does reading of input from terminal w/o the editing features
> -   provided by the readline library.  */
> +   provided by the readline library.  Calls the line input handler
> +   once we have a whole input line.  */
>  
> -/* NOTE: 1999-04-30 Asynchronous version of gdb_readline; gdb_readline
> -   will become obsolete when the event loop is made the default
> -   execution for gdb.  */
>  void
> -gdb_readline2 (gdb_client_data client_data)
> +gdb_readline_callback_no_editing (gdb_client_data client_data)
>  {
>    int c;
>    char *result;
> @@ -728,11 +727,12 @@ gdb_readline2 (gdb_client_data client_data)
>    result = (char *) xmalloc (result_size);
>  
>    /* We still need the while loop here, even though it would seem
> -     obvious to invoke gdb_readline2 at every character entered.  If
> -     not using the readline library, the terminal is in cooked mode,
> -     which sends the characters all at once.  Poll will notice that the
> -     input fd has changed state only after enter is pressed.  At this
> -     point we still need to fetch all the chars entered.  */
> +     obvious to invoke gdb_readline_callback_no_editing at every
> +     character entered.  If not using the readline library, the
> +     terminal is in cooked mode, which sends the characters all at
> +     once.  Poll will notice that the input fd has changed state only
> +     after enter is pressed.  At this point we still need to fetch all
> +     the chars entered.  */
>  
>    while (1)
>      {
> @@ -1055,7 +1055,7 @@ gdb_setup_readline (void)
>    else
>      {
>        async_command_editing_p = 0;
> -      call_readline = gdb_readline2;
> +      call_readline = gdb_readline_callback_no_editing;
>      }
>    
>    /* When readline has read an end-of-line character, it passes the
> diff --git a/gdb/event-top.h b/gdb/event-top.h
> index 4f20770..64c6fdf 100644
> --- a/gdb/event-top.h
> +++ b/gdb/event-top.h
> @@ -44,7 +44,6 @@ extern void handle_stop_sig (int sig);
>  #endif
>  extern void handle_sigint (int sig);
>  extern void handle_sigterm (int sig);
> -extern void gdb_readline2 (void *client_data);
>  extern void async_request_quit (void *arg);
>  extern void stdin_event_handler (int error, void *client_data);
>  extern void async_disable_stdin (void);
> @@ -62,6 +61,7 @@ extern void (*input_handler) (char *);
>  extern int input_fd;
>  extern void (*after_char_processing_hook) (void);
>  extern int call_stdin_event_handler_again_p;
> +extern void gdb_readline_callback_no_editing (void *client_data);
>  
>  /* Wrappers for rl_callback_handler_remove and
>     rl_callback_handler_install that keep track of whether the callback
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 7f42367..80c8259 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -181,7 +181,7 @@ mi_interpreter_resume (void *data)
>  
>    /* These overwrite some of the initialization done in
>       _intialize_event_loop.  */
> -  call_readline = gdb_readline2;
> +  call_readline = gdb_readline_callback_no_editing;
>    input_handler = mi_execute_command_input_handler;
>    async_command_editing_p = 0;
>    /* FIXME: This is a total hack for now.  PB's use of the MI
> -- 
> 1.9.3

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 03/10] gdb_readline2 -> gdb_readline_callback_no_editing

[Pedro Alves]

On 02/18/2016 09:59 PM, Sergio Durigan Junior wrote:
> On Thursday, February 18 2016, Pedro Alves wrote:
>
>> The "2" in "gdb_readline2" doesn't really convey much.  Rename for clarity.
>
> Any reason to put the "callback" in the middle of the name?  I usually
> prefer to put it in the end.  "gdb_readline_no_editing_callback" seems
> better to me.

No big reason, I've changed it.

> Other than that, LGTM.

Below's what I pushed.

Thanks,
Pedro Alves

From c70061cf94691182484924c79cbbdd2203ef92d5 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Mar 2016 18:24:59 +0000
Subject: [PATCH 03/10] gdb_readline2 -> gdb_readline_no_editing_callback

The "2" in "gdb_readline2" doesn't really convey much.  Rename for
clarity.

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

	* event-top.c (gdb_readline2): Rename to ...
	(gdb_readline_no_editing_callback): ... this.
	(change_line_handler, stdin_event_handler)
	(gdb_setup_readline): Adjust.
	* event-top.h (gdb_readline2): Rename to ...
	(gdb_readline_no_editing_callback): ... this, and move closer to
	other readline-related declarations.
	* mi/mi-interp.c (mi_interpreter_resume): Adjust.
---
 gdb/ChangeLog      | 11 +++++++++++
 gdb/event-top.c    | 36 ++++++++++++++++++------------------
 gdb/event-top.h    |  2 +-
 gdb/mi/mi-interp.c |  2 +-
 4 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d220a02..09b3826 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,16 @@
 2016-03-09  Pedro Alves  <palves@redhat.com>
 
+	* event-top.c (gdb_readline2): Rename to ...
+	(gdb_readline_no_editing_callback): ... this.
+	(change_line_handler, stdin_event_handler)
+	(gdb_setup_readline): Adjust.
+	* event-top.h (gdb_readline2): Rename to ...
+	(gdb_readline_no_editing_callback): ... this, and move closer to
+	other readline-related declarations.
+	* mi/mi-interp.c (mi_interpreter_resume): Adjust.
+
+2016-03-09  Pedro Alves  <palves@redhat.com>
+
 	* top.c (window_hook): Delete.
 	(command_loop): Remove references to window_hook.
 
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 2309cce..e3fa589 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -189,9 +189,9 @@ cli_command_loop (void *data)
 /* Change the function to be invoked every time there is a character
    ready on stdin.  This is used when the user sets the editing off,
    therefore bypassing readline, and letting gdb handle the input
-   itself, via gdb_readline2.  Also it is used in the opposite case in
-   which the user sets editing on again, by restoring readline
-   handling of the input.  */
+   itself, via gdb_readline_no_editing_callback.  Also it is used in
+   the opposite case in which the user sets editing on again, by
+   restoring readline handling of the input.  */
 static void
 change_line_handler (void)
 {
@@ -209,9 +209,9 @@ change_line_handler (void)
     }
   else
     {
-      /* Turn off editing by using gdb_readline2.  */
+      /* Turn off editing by using gdb_readline_no_editing_callback.  */
       gdb_rl_callback_handler_remove ();
-      call_readline = gdb_readline2;
+      call_readline = gdb_readline_no_editing_callback;
 
       /* Set up the command handler as well, in case we are called as
          first thing from .gdbinit.  */
@@ -410,8 +410,9 @@ top_level_prompt (void)
 
 /* When there is an event ready on the stdin file desriptor, instead
    of calling readline directly throught the callback function, or
-   instead of calling gdb_readline2, give gdb a chance to detect
-   errors and do something.  */
+   instead of calling gdb_readline_no_editing_callback, give gdb a
+   chance to detect errors and do something.  */
+
 void
 stdin_event_handler (int error, gdb_client_data client_data)
 {
@@ -699,13 +700,11 @@ command_line_handler (char *rl)
 }
 
 /* Does reading of input from terminal w/o the editing features
-   provided by the readline library.  */
+   provided by the readline library.  Calls the line input handler
+   once we have a whole input line.  */
 
-/* NOTE: 1999-04-30 Asynchronous version of gdb_readline; gdb_readline
-   will become obsolete when the event loop is made the default
-   execution for gdb.  */
 void
-gdb_readline2 (gdb_client_data client_data)
+gdb_readline_no_editing_callback (gdb_client_data client_data)
 {
   int c;
   char *result;
@@ -728,11 +727,12 @@ gdb_readline2 (gdb_client_data client_data)
   result = (char *) xmalloc (result_size);
 
   /* We still need the while loop here, even though it would seem
-     obvious to invoke gdb_readline2 at every character entered.  If
-     not using the readline library, the terminal is in cooked mode,
-     which sends the characters all at once.  Poll will notice that the
-     input fd has changed state only after enter is pressed.  At this
-     point we still need to fetch all the chars entered.  */
+     obvious to invoke gdb_readline_no_editing_callback at every
+     character entered.  If not using the readline library, the
+     terminal is in cooked mode, which sends the characters all at
+     once.  Poll will notice that the input fd has changed state only
+     after enter is pressed.  At this point we still need to fetch all
+     the chars entered.  */
 
   while (1)
     {
@@ -1055,7 +1055,7 @@ gdb_setup_readline (void)
   else
     {
       async_command_editing_p = 0;
-      call_readline = gdb_readline2;
+      call_readline = gdb_readline_no_editing_callback;
     }
   
   /* When readline has read an end-of-line character, it passes the
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 4f20770..f9bd7b9 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -44,7 +44,6 @@ extern void handle_stop_sig (int sig);
 #endif
 extern void handle_sigint (int sig);
 extern void handle_sigterm (int sig);
-extern void gdb_readline2 (void *client_data);
 extern void async_request_quit (void *arg);
 extern void stdin_event_handler (int error, void *client_data);
 extern void async_disable_stdin (void);
@@ -62,6 +61,7 @@ extern void (*input_handler) (char *);
 extern int input_fd;
 extern void (*after_char_processing_hook) (void);
 extern int call_stdin_event_handler_again_p;
+extern void gdb_readline_no_editing_callback (void *client_data);
 
 /* Wrappers for rl_callback_handler_remove and
    rl_callback_handler_install that keep track of whether the callback
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 7f42367..d02012c 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -181,7 +181,7 @@ mi_interpreter_resume (void *data)
 
   /* These overwrite some of the initialization done in
      _intialize_event_loop.  */
-  call_readline = gdb_readline2;
+  call_readline = gdb_readline_no_editing_callback;
   input_handler = mi_execute_command_input_handler;
   async_command_editing_p = 0;
   /* FIXME: This is a total hack for now.  PB's use of the MI
-- 
2.5.0

[PATCH 05/10] Update prompt_for_continue comments

[Pedro Alves]

These comments are out of date -- we no longer call gdb_readline.  And
I think that mentioning the event loop is more useful here than
whatever GO32 issue had with gdb_readline, which may even no longer be
an issue.

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* utils.c (prompt_for_continue): Update comments.
---
 gdb/utils.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index e34c12e..a4aa8f5 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1808,7 +1808,9 @@ set_screen_width_and_height (int width, int height)
 }
 
 /* Wait, so the user can read what's on the screen.  Prompt the user
-   to continue by pressing RETURN.  */
+   to continue by pressing RETURN. 'q' is also provided because
+   telling users what to do in the prompt is more user-friendly than
+   expecting them to think of Ctrl-C/SIGINT.  */
 
 static void
 prompt_for_continue (void)
@@ -1829,9 +1831,9 @@ prompt_for_continue (void)
   if (annotation_level > 1)
     strcat (cont_prompt, "\n\032\032prompt-for-continue\n");
 
-  /* We must do this *before* we call gdb_readline, else it will eventually
-     call us -- thinking that we're trying to print beyond the end of the 
-     screen.  */
+  /* We must do this *before* we call gdb_readline_wrapper, else it
+     will eventually call us -- thinking that we're trying to print
+     beyond the end of the screen.  */
   reinitialize_more_filter ();
 
   immediate_quit++;
@@ -1840,16 +1842,8 @@ prompt_for_continue (void)
   /* We'll need to handle input.  */
   target_terminal_ours ();
 
-  /* On a real operating system, the user can quit with SIGINT.
-     But not on GO32.
-
-     'q' is provided on all systems so users don't have to change habits
-     from system to system, and because telling them what to do in
-     the prompt is more user-friendly than expecting them to think of
-     SIGINT.  */
-  /* Call readline, not gdb_readline, because GO32 readline handles control-C
-     whereas control-C to gdb_readline will cause the user to get dumped
-     out to DOS.  */
+  /* Call gdb_readline_wrapper, not readline, in order to keep an
+     event loop running.  */
   ignore = gdb_readline_wrapper (cont_prompt);
 
   /* Add time spend in this routine to prompt_for_continue_wait_time.  */
-- 
1.9.3

Re: [PATCH 05/10] Update prompt_for_continue comments

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

>  /* Wait, so the user can read what's on the screen.  Prompt the user
> -   to continue by pressing RETURN.  */
> +   to continue by pressing RETURN. 'q' is also provided because

One more space before 'q'.

> +   telling users what to do in the prompt is more user-friendly than
> +   expecting them to think of Ctrl-C/SIGINT.  */

-- 
Yao (齐尧)

Re: [PATCH 05/10] Update prompt_for_continue comments

[Pedro Alves]

On 02/22/2016 02:01 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>>   /* Wait, so the user can read what's on the screen.  Prompt the user
>> -   to continue by pressing RETURN.  */
>> +   to continue by pressing RETURN. 'q' is also provided because
>
> One more space before 'q'.

Fixed and pushed.

>
>> +   telling users what to do in the prompt is more user-friendly than
>> +   expecting them to think of Ctrl-C/SIGINT.  */
>

Thanks,
Pedro Alves

[PATCH 10/10] Command line input handling TLC

[Pedro Alves]

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

 - Use "struct buffer" more.

 - Split out the responsability 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-02-18  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/event-top.c | 340 +++++++++++++++++++++++++++-----------------------------
 gdb/event-top.h |   2 +
 gdb/top.c       | 170 +++++-----------------------
 gdb/top.h       |   6 +
 4 files changed, 200 insertions(+), 318 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 221e242..265e511 100644
--- a/gdb/event-top.c
+++ b/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_callback_no_editing, give gdb a
@@ -436,152 +439,122 @@ async_disable_stdin (void)
 }
 \f
 
-/* Handles a gdb command.  This function is called by
-   command_line_handler, which has processed one or more input lines
-   into COMMAND.  */
-/* NOTE: 1999-04-30 This is the asynchronous version of the command_loop
-   function.  The command_loop function will be obsolete when we
-   switch to use the event loop at every execution of gdb.  */
-static void
+/* Handle a gdb command line.  This function is called when
+   handle_line_of_input has concatenated one or more input lines into
+   a whole command.  */
+
+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);
 
-  execute_command (command, instream == stdin);
+  /* 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 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
-   mechanism within the readline library.  Deal with incomplete
-   commands as well, by saving the partial input in a global
-   buffer.  */
+/* Append RL, an input line returned by readline or one of its
+   emulations, to CMD_LINE_BUFFER.  Returns false if more input is
+   expected (input line ends in a backslash), true if we have a whole
+   command line ready to be processed by the command interpreter.
+   Takes ownership of RL.  */
 
-/* NOTE: 1999-04-30 This is the asynchronous version of the
-   command_line_input function; command_line_input will become
-   obsolete once we use the event loop as the default mechanism in
-   GDB.  */
-static void
-command_line_handler (char *rl)
+static char *
+command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
 {
-  static char *linebuffer = 0;
-  static unsigned linelength = 0;
-  char *p;
-  char *p1;
-  char *nline;
-  int repeat = (instream == stdin);
+  char *cmd;
+  size_t len;
 
-  if (annotation_level > 1 && instream == stdin)
-    printf_unfiltered (("\n\032\032post-prompt\n"));
+  len = strlen (rl);
 
-  if (linebuffer == 0)
+  if (len > 0 && rl[len - 1] == '\\')
     {
-      linelength = 80;
-      linebuffer = (char *) xmalloc (linelength);
-      linebuffer[0] = '\0';
+      /* Don't copy the backslash and wait for more.  */
+      buffer_grow (cmd_line_buffer, rl, len - 1);
+      cmd = NULL;
     }
-
-  p = linebuffer;
-
-  if (more_to_come)
+  else
     {
-      strcpy (linebuffer, readline_input_state.linebuffer);
-      p = readline_input_state.linebuffer_ptr;
-      xfree (readline_input_state.linebuffer);
-      more_to_come = 0;
+      /* Copy whole line including terminating null, and we're
+	 done.  */
+      buffer_grow (cmd_line_buffer, rl, len + 1);
+      cmd = cmd_line_buffer->buffer;
     }
 
-#ifdef STOP_SIGNAL
-  if (job_control)
-    signal (STOP_SIGNAL, handle_stop_sig);
-#endif
+  /* Allocated in readline.  */
+  xfree (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.  */
-  wrap_here ("");
-  gdb_flush (gdb_stdout);
-  gdb_flush (gdb_stderr);
+  return cmd;
+}
 
-  if (source_file_name != NULL)
-    ++source_line_number;
+/* Handle a line of input coming from readline.
 
-  /* 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++;
+   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.
 
-  xfree (rl);			/* Allocated in readline.  */
+   Returns EOF on end of file.
 
-  if (p > linebuffer && *(p - 1) == '\\')
-    {
-      *p = '\0';
-      p--;			/* Put on top of '\'.  */
+   If REPEAT, handle command repetitions:
 
-      readline_input_state.linebuffer = xstrdup (linebuffer);
-      readline_input_state.linebuffer_ptr = p;
+     - 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.
 
-      /* 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;
-    }
+     - OTOH, if the input command line IS empty, return the previously
+       saved command instead of the empty input line.
+*/
 
-#ifdef STOP_SIGNAL
-  if (job_control)
-    signal (STOP_SIGNAL, SIG_DFL);
-#endif
+char *
+handle_line_of_input (struct buffer *cmd_line_buffer,
+		      char *rl, int repeat, char *annotation_suffix)
+{
+  char *p1;
+  char *cmd;
+
+  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-"));
+      puts_unfiltered (annotation_suffix);
+      printf_unfiltered (("\n"));
+    }
 
-#define SERVER_COMMAND_LENGTH 7
-  server_command =
-    (p - linebuffer > SERVER_COMMAND_LENGTH)
-    && strncmp (linebuffer, "server ", SERVER_COMMAND_LENGTH) == 0;
-  if (server_command)
+#define SERVER_COMMAND_PREFIX "server "
+  if (startswith (cmd, SERVER_COMMAND_PREFIX))
     {
-      /* 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;
+      /* Note that we don't set `saved_command_line'.  Between this
+         and the check in dont_repeat, this insures that repeating
+         will still do the right thing.  */
+      return cmd + strlen (SERVER_COMMAND_PREFIX);
     }
 
   /* 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 (linebuffer, &history_value);
+      expanded = history_expand (cmd, &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)
-	    {
-	      linelength = strlen (history_value) + 1;
-	      linebuffer = (char *) xrealloc (linebuffer, linelength);
-	    }
-	  strcpy (linebuffer, history_value);
-	  p = linebuffer + strlen (linebuffer);
+
+	  /* history_expand returns an allocated string.  Just replace
+	     our buffer with it.  */
+	  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;
 	}
-      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 && *p != '\\')
-    {
-      command_handler (saved_command_line);
-      display_gdb_prompt (0);
-      return;
-    }
+     previous command, return the previously saved command.  */
+  for (p1 = cmd; *p1 == ' ' || *p1 == '\t'; p1++)
+    ;
+  if (repeat && *p1 == '\0')
+    return saved_command_line;
+
+  /* Add command to history if appropriate.  Note: lines consisting
+     solely of comments are also 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 (*cmd != '\0' && input_from_terminal_p ())
+    gdb_add_history (cmd);
 
-  for (p1 = linebuffer; *p1 == ' ' || *p1 == '\t'; p1++);
-  if (repeat && !*p1)
+  /* Save into global buffer if appropriate.  */
+  if (repeat)
     {
-      command_handler (saved_command_line);
-      display_gdb_prompt (0);
-      return;
+      xfree (saved_command_line);
+      saved_command_line = xstrdup (cmd);
+      return saved_command_line;
     }
+  else
+    return cmd;
+}
 
-  *p = 0;
+/* 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.
 
-  /* Add line to history if appropriate.  */
-  if (*linebuffer && input_from_terminal_p ())
-    gdb_add_history (linebuffer);
+   NOTE: This is the asynchronous version of the command_line_input
+   function.  */
 
-  /* 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.  */
+void
+command_line_handler (char *rl)
+{
+  struct buffer *line_buffer = get_command_line_buffer ();
+  char *cmd;
 
-  /* Save into global buffer if appropriate.  */
-  if (repeat)
+  cmd = handle_line_of_input (line_buffer, rl, instream == stdin, "prompt");
+  if (cmd == (char *) EOF)
     {
-      xfree (saved_command_line);
-      saved_command_line = xstrdup (linebuffer);
-      if (!more_to_come)
-	{
-	  command_handler (saved_command_line);
-	  display_gdb_prompt (0);
-	}
-      return;
+      /* 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);
     }
-
-  command_handler (linebuffer);
-  display_gdb_prompt (0);
-  return;
 }
 
 /* Does reading of input from terminal w/o the editing features
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 1a79d62..44e2041 100644
--- a/gdb/event-top.h
+++ b/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>
diff --git a/gdb/top.c b/gdb/top.c
index 1a5c3f9..89fe832 100644
--- a/gdb/top.c
+++ b/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 ();
-      if (instream == stdin)
-	reinitialize_more_filter ();
-      old_chain = make_cleanup (null_cleanup, 0);
+      char *command;
 
       /* 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)
-	{
-	  do_cleanups (old_chain);
-	  return;
-	}
-
-      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);
+      if (command == NULL)
+	return;
+      command_handler (command);
     }
 }
 \f
@@ -1016,32 +996,26 @@ gdb_safe_append_history (void)
   do_cleanups (old_chain);
 }
 
-/* Read one line from the command input stream `instream'
-   into the local static buffer `linebuffer' (whose current length
-   is `linelength').
-   The buffer is made bigger as necessary.
-   Returns the address of the start of the line.
+/* Read one line from the command input stream `instream' into a local
+   static buffer.  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
-   is copied into the file line saver (global var char *line,
-   length linesize) so that it can be duplicated.
+   *If* the instream == stdin & stdin is a terminal, the line read is
+   copied into the global 'saved_command_line' so that it can be
+   repeated.
 
-   This routine either uses fancy command line editing or
-   simple input as the user has requested.  */
+   This routine either uses fancy command line editing or simple input
+   as the user has requested.  */
 
 char *
 command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
 {
-  static char *linebuffer = 0;
-  static unsigned linelength = 0;
+  static struct buffer cmd_line_buffer;
+  static int cmd_line_buffer_initialized;
   const char *prompt = prompt_arg;
-  char *p;
-  char *p1;
-  char *rl;
-  char *nline;
-  char got_eof = 0;
+  char *cmd;
 
   /* 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;
-      linebuffer = (char *) xmalloc (linelength);
+      buffer_init (&cmd_line_buffer);
+      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-");
-	  puts_unfiltered (annotation_suffix);
-	  puts_unfiltered ("\n");
-	}
-
-      if (!rl || rl == (char *) EOF)
-	{
-	  got_eof = 1;
+	  cmd = NULL;
 	  break;
 	}
-      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) != '\\')
+      if (cmd != NULL)
 	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 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;
+  return cmd;
 }
 \f
 /* Print the GDB banner.  */
diff --git a/gdb/top.h b/gdb/top.h
index f3b080b..a498f39 100644
--- a/gdb/top.h
+++ b/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
-- 
1.9.3

Re: [PATCH 10/10] Command line input handling TLC

[Simon Marchi]

On 16-02-18 12:40 PM, Pedro Alves wrote:
> I didn't manage to usefully split this further into smaller
> independent pieces, so:
> 
>  - Use "struct buffer" more.
> 
>  - Split out the responsability of composing a complete command line

responsibility

> -/* 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.  */
> +/* Append RL, an input line returned by readline or one of its
> +   emulations, to CMD_LINE_BUFFER.  Returns false if more input is
> +   expected (input line ends in a backslash), true if we have a whole
> +   command line ready to be processed by the command interpreter.
> +   Takes ownership of RL.  */
>  
> -/* NOTE: 1999-04-30 This is the asynchronous version of the
> -   command_line_input function; command_line_input will become
> -   obsolete once we use the event loop as the default mechanism in
> -   GDB.  */
> -static void
> -command_line_handler (char *rl)
> +static char *
> +command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)

The comment seems outdated, the function doesn't return true/false, but NULL
or non-NULL. I suppose it did return true/false in an earlier version of the
patch :).

Re: [PATCH 10/10] Command line input handling TLC

[Pedro Alves]

On 02/19/2016 04:01 PM, Simon Marchi wrote:
> On 16-02-18 12:40 PM, Pedro Alves wrote:
>> I didn't manage to usefully split this further into smaller
>> independent pieces, so:
>>
>>   - Use "struct buffer" more.
>>
>>   - Split out the responsability of composing a complete command line
> 
> responsibility

Fixed.

> 
>> -/* 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.  */
>> +/* Append RL, an input line returned by readline or one of its
>> +   emulations, to CMD_LINE_BUFFER.  Returns false if more input is
>> +   expected (input line ends in a backslash), true if we have a whole
>> +   command line ready to be processed by the command interpreter.
>> +   Takes ownership of RL.  */
>>   
>> -/* NOTE: 1999-04-30 This is the asynchronous version of the
>> -   command_line_input function; command_line_input will become
>> -   obsolete once we use the event loop as the default mechanism in
>> -   GDB.  */
>> -static void
>> -command_line_handler (char *rl)
>> +static char *
>> +command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
> 
> The comment seems outdated, the function doesn't return true/false, but NULL
> or non-NULL. I suppose it did return true/false in an earlier version of the
> patch :).

Whoops, indeed!  It now reads:

/* Append RL, an input line returned by readline or one of its
   emulations, to CMD_LINE_BUFFER.  Returns the command line if we
   have a whole command line ready to be processed by the command
   interpreter or NULL if the command line isn't complete yet (input
   line ends in a backslash).  Takes ownership of RL.  */

Below's what I pushed.

From b69d38afdea34e4fecab5ea47ffe1e594e0b6233 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Mar 2016 18:25:00 +0000
Subject: [PATCH 10/10] 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   |  19 ++++
 gdb/event-top.c | 340 +++++++++++++++++++++++++++-----------------------------
 gdb/event-top.h |   2 +
 gdb/top.c       | 170 +++++-----------------------
 gdb/top.h       |   6 +
 5 files changed, 219 insertions(+), 318 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index bc2e99e..37118c5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,24 @@
 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
 	of xrealloc + strcpy.
 	* main.c (captured_main): Use xstrdup instead of xmalloc plus
diff --git a/gdb/event-top.c b/gdb/event-top.c
index f112c52..eb4f0b9 100644
--- a/gdb/event-top.c
+++ b/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)
 }
 \f
 
-/* Handles a gdb command.  This function is called by
-   command_line_handler, which has processed one or more input lines
-   into COMMAND.  */
-/* NOTE: 1999-04-30 This is the asynchronous version of the command_loop
-   function.  The command_loop function will be obsolete when we
-   switch to use the event loop at every execution of gdb.  */
-static void
+/* Handle a gdb command line.  This function is called when
+   handle_line_of_input has concatenated one or more input lines into
+   a whole command.  */
+
+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);
 
-  execute_command (command, instream == stdin);
+  /* 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 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
-   mechanism within the readline library.  Deal with incomplete
-   commands as well, by saving the partial input in a global
-   buffer.  */
+/* Append RL, an input line returned by readline or one of its
+   emulations, to CMD_LINE_BUFFER.  Returns the command line if we
+   have a whole command line ready to be processed by the command
+   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
-   command_line_input function; command_line_input will become
-   obsolete once we use the event loop as the default mechanism in
-   GDB.  */
-static void
-command_line_handler (char *rl)
+static char *
+command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
 {
-  static char *linebuffer = 0;
-  static unsigned linelength = 0;
-  char *p;
-  char *p1;
-  char *nline;
-  int repeat = (instream == stdin);
+  char *cmd;
+  size_t len;
 
-  if (annotation_level > 1 && instream == stdin)
-    printf_unfiltered (("\n\032\032post-prompt\n"));
+  len = strlen (rl);
 
-  if (linebuffer == 0)
+  if (len > 0 && rl[len - 1] == '\\')
     {
-      linelength = 80;
-      linebuffer = (char *) xmalloc (linelength);
-      linebuffer[0] = '\0';
+      /* Don't copy the backslash and wait for more.  */
+      buffer_grow (cmd_line_buffer, rl, len - 1);
+      cmd = NULL;
     }
-
-  p = linebuffer;
-
-  if (more_to_come)
+  else
     {
-      strcpy (linebuffer, readline_input_state.linebuffer);
-      p = readline_input_state.linebuffer_ptr;
-      xfree (readline_input_state.linebuffer);
-      more_to_come = 0;
+      /* Copy whole line including terminating null, and we're
+	 done.  */
+      buffer_grow (cmd_line_buffer, rl, len + 1);
+      cmd = cmd_line_buffer->buffer;
     }
 
-#ifdef STOP_SIGNAL
-  if (job_control)
-    signal (STOP_SIGNAL, handle_stop_sig);
-#endif
+  /* Allocated in readline.  */
+  xfree (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.  */
-  wrap_here ("");
-  gdb_flush (gdb_stdout);
-  gdb_flush (gdb_stderr);
+  return cmd;
+}
 
-  if (source_file_name != NULL)
-    ++source_line_number;
+/* Handle a line of input coming from readline.
 
-  /* 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++;
+   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.
 
-  xfree (rl);			/* Allocated in readline.  */
+   Returns EOF on end of file.
 
-  if (p > linebuffer && *(p - 1) == '\\')
-    {
-      *p = '\0';
-      p--;			/* Put on top of '\'.  */
+   If REPEAT, handle command repetitions:
 
-      readline_input_state.linebuffer = xstrdup (linebuffer);
-      readline_input_state.linebuffer_ptr = p;
+     - 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.
 
-      /* 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;
-    }
+     - OTOH, if the input command line IS empty, return the previously
+       saved command instead of the empty input line.
+*/
 
-#ifdef STOP_SIGNAL
-  if (job_control)
-    signal (STOP_SIGNAL, SIG_DFL);
-#endif
+char *
+handle_line_of_input (struct buffer *cmd_line_buffer,
+		      char *rl, int repeat, char *annotation_suffix)
+{
+  char *p1;
+  char *cmd;
+
+  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-"));
+      puts_unfiltered (annotation_suffix);
+      printf_unfiltered (("\n"));
+    }
 
-#define SERVER_COMMAND_LENGTH 7
-  server_command =
-    (p - linebuffer > SERVER_COMMAND_LENGTH)
-    && strncmp (linebuffer, "server ", SERVER_COMMAND_LENGTH) == 0;
-  if (server_command)
+#define SERVER_COMMAND_PREFIX "server "
+  if (startswith (cmd, SERVER_COMMAND_PREFIX))
     {
-      /* 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;
+      /* Note that we don't set `saved_command_line'.  Between this
+         and the check in dont_repeat, this insures that repeating
+         will still do the right thing.  */
+      return cmd + strlen (SERVER_COMMAND_PREFIX);
     }
 
   /* 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 (linebuffer, &history_value);
+      expanded = history_expand (cmd, &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)
-	    {
-	      linelength = strlen (history_value) + 1;
-	      linebuffer = (char *) xrealloc (linebuffer, linelength);
-	    }
-	  strcpy (linebuffer, history_value);
-	  p = linebuffer + strlen (linebuffer);
+
+	  /* history_expand returns an allocated string.  Just replace
+	     our buffer with it.  */
+	  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;
 	}
-      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 && *p != '\\')
-    {
-      command_handler (saved_command_line);
-      display_gdb_prompt (0);
-      return;
-    }
+     previous command, return the previously saved command.  */
+  for (p1 = cmd; *p1 == ' ' || *p1 == '\t'; p1++)
+    ;
+  if (repeat && *p1 == '\0')
+    return saved_command_line;
+
+  /* Add command to history if appropriate.  Note: lines consisting
+     solely of comments are also 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 (*cmd != '\0' && input_from_terminal_p ())
+    gdb_add_history (cmd);
 
-  for (p1 = linebuffer; *p1 == ' ' || *p1 == '\t'; p1++);
-  if (repeat && !*p1)
+  /* Save into global buffer if appropriate.  */
+  if (repeat)
     {
-      command_handler (saved_command_line);
-      display_gdb_prompt (0);
-      return;
+      xfree (saved_command_line);
+      saved_command_line = xstrdup (cmd);
+      return saved_command_line;
     }
+  else
+    return cmd;
+}
 
-  *p = 0;
+/* 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.
 
-  /* Add line to history if appropriate.  */
-  if (*linebuffer && input_from_terminal_p ())
-    gdb_add_history (linebuffer);
+   NOTE: This is the asynchronous version of the command_line_input
+   function.  */
 
-  /* 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.  */
+void
+command_line_handler (char *rl)
+{
+  struct buffer *line_buffer = get_command_line_buffer ();
+  char *cmd;
 
-  /* Save into global buffer if appropriate.  */
-  if (repeat)
+  cmd = handle_line_of_input (line_buffer, rl, instream == stdin, "prompt");
+  if (cmd == (char *) EOF)
     {
-      xfree (saved_command_line);
-      saved_command_line = xstrdup (linebuffer);
-      if (!more_to_come)
-	{
-	  command_handler (saved_command_line);
-	  display_gdb_prompt (0);
-	}
-      return;
+      /* 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);
     }
-
-  command_handler (linebuffer);
-  display_gdb_prompt (0);
-  return;
 }
 
 /* Does reading of input from terminal w/o the editing features
diff --git a/gdb/event-top.h b/gdb/event-top.h
index b8c3b11..bef2530 100644
--- a/gdb/event-top.h
+++ b/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>
diff --git a/gdb/top.c b/gdb/top.c
index 1a5c3f9..89fe832 100644
--- a/gdb/top.c
+++ b/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 ();
-      if (instream == stdin)
-	reinitialize_more_filter ();
-      old_chain = make_cleanup (null_cleanup, 0);
+      char *command;
 
       /* 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)
-	{
-	  do_cleanups (old_chain);
-	  return;
-	}
-
-      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);
+      if (command == NULL)
+	return;
+      command_handler (command);
     }
 }
 \f
@@ -1016,32 +996,26 @@ gdb_safe_append_history (void)
   do_cleanups (old_chain);
 }
 
-/* Read one line from the command input stream `instream'
-   into the local static buffer `linebuffer' (whose current length
-   is `linelength').
-   The buffer is made bigger as necessary.
-   Returns the address of the start of the line.
+/* Read one line from the command input stream `instream' into a local
+   static buffer.  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
-   is copied into the file line saver (global var char *line,
-   length linesize) so that it can be duplicated.
+   *If* the instream == stdin & stdin is a terminal, the line read is
+   copied into the global 'saved_command_line' so that it can be
+   repeated.
 
-   This routine either uses fancy command line editing or
-   simple input as the user has requested.  */
+   This routine either uses fancy command line editing or simple input
+   as the user has requested.  */
 
 char *
 command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
 {
-  static char *linebuffer = 0;
-  static unsigned linelength = 0;
+  static struct buffer cmd_line_buffer;
+  static int cmd_line_buffer_initialized;
   const char *prompt = prompt_arg;
-  char *p;
-  char *p1;
-  char *rl;
-  char *nline;
-  char got_eof = 0;
+  char *cmd;
 
   /* 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;
-      linebuffer = (char *) xmalloc (linelength);
+      buffer_init (&cmd_line_buffer);
+      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-");
-	  puts_unfiltered (annotation_suffix);
-	  puts_unfiltered ("\n");
-	}
-
-      if (!rl || rl == (char *) EOF)
-	{
-	  got_eof = 1;
+	  cmd = NULL;
 	  break;
 	}
-      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) != '\\')
+      if (cmd != NULL)
 	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 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;
+  return cmd;
 }
 \f
 /* Print the GDB banner.  */
diff --git a/gdb/top.h b/gdb/top.h
index f3b080b..a498f39 100644
--- a/gdb/top.h
+++ b/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
-- 
2.5.0

Re: [PATCH 10/10] Command line input handling TLC

[Luis Machado]

On 02/18/2016 03:40 PM, Pedro Alves wrote:
> I didn't manage to usefully split this further into smaller
> independent pieces, so:
>
>   - Use "struct buffer" more.
>
>   - Split out the responsability 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-02-18  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/event-top.c | 340 +++++++++++++++++++++++++++-----------------------------
>   gdb/event-top.h |   2 +
>   gdb/top.c       | 170 +++++-----------------------
>   gdb/top.h       |   6 +
>   4 files changed, 200 insertions(+), 318 deletions(-)
>

This and the rest of the series look good to me. That's a nice cleanup.

Thanks,
Luis

[PATCH 04/10] Eliminate async_annotation_suffix

[Pedro Alves]

The comments and existence of this global are a bit of misleading
obfuscation, since this is only ever used to print the prompt
annotation, and never changes.  Just hardcode "prompt" where
necessary, as done for most other annotations.

gdb/ChangeLog:
2016-02-18  Pedro Alves  <palves@redhat.com>

	* event-top.c (async_annotation_suffix): Delete.
	(top_level_prompt, command_line_handler): Don't use
	'async_annotation_suffix' and simplify.
	* event-top.h (async_annotation_suffix): Delete declaration.
	(init_main): Remove reference to 'async_annotation_suffix'.
---
 gdb/event-top.c | 46 ++++++++--------------------------------------
 gdb/event-top.h |  1 -
 gdb/top.c       |  4 ----
 3 files changed, 8 insertions(+), 43 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 35830119..ee44197 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -107,10 +107,6 @@ void (*call_readline) (gdb_client_data);
    loop as default engine, and event-top.c is merged into top.c.  */
 int async_command_editing_p;
 
-/* This is the annotation suffix that will be used when the
-   annotation_level is 2.  */
-char *async_annotation_suffix;
-
 /* This is used to display the notification of the completion of an
    asynchronous execution command.  */
 int exec_done_display_p = 0;
@@ -363,49 +359,27 @@ display_gdb_prompt (const char *new_prompt)
 static char *
 top_level_prompt (void)
 {
-  char *prefix;
-  char *prompt = NULL;
-  char *suffix;
-  char *composed_prompt;
-  size_t prompt_length;
+  char *prompt;
 
   /* Give observers a chance of changing the prompt.  E.g., the python
      `gdb.prompt_hook' is installed as an observer.  */
   observer_notify_before_prompt (get_prompt ());
 
-  prompt = xstrdup (get_prompt ());
+  prompt = get_prompt ();
 
   if (annotation_level >= 2)
     {
       /* Prefix needs to have new line at end.  */
-      prefix = (char *) alloca (strlen (async_annotation_suffix) + 10);
-      strcpy (prefix, "\n\032\032pre-");
-      strcat (prefix, async_annotation_suffix);
-      strcat (prefix, "\n");
+      const char prefix[] = "\n\032\032pre-prompt\n";
 
       /* Suffix needs to have a new line at end and \032 \032 at
 	 beginning.  */
-      suffix = (char *) alloca (strlen (async_annotation_suffix) + 6);
-      strcpy (suffix, "\n\032\032");
-      strcat (suffix, async_annotation_suffix);
-      strcat (suffix, "\n");
-    }
-  else
-    {
-      prefix = "";
-      suffix = "";
-    }
+      const char suffix[] = "\n\032\032prompt\n";
 
-  prompt_length = strlen (prefix) + strlen (prompt) + strlen (suffix);
-  composed_prompt = (char *) xmalloc (prompt_length + 1);
-
-  strcpy (composed_prompt, prefix);
-  strcat (composed_prompt, prompt);
-  strcat (composed_prompt, suffix);
-
-  xfree (prompt);
+      return concat (prefix, prompt, suffix, NULL);
+    }
 
-  return composed_prompt;
+  return xstrdup (prompt);
 }
 
 /* When there is an event ready on the stdin file desriptor, instead
@@ -517,11 +491,7 @@ command_line_handler (char *rl)
   int repeat = (instream == stdin);
 
   if (annotation_level > 1 && instream == stdin)
-    {
-      printf_unfiltered (("\n\032\032post-"));
-      puts_unfiltered (async_annotation_suffix);
-      printf_unfiltered (("\n"));
-    }
+    printf_unfiltered (("\n\032\032post-prompt\n"));
 
   if (linebuffer == 0)
     {
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 64c6fdf..1a79d62 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -54,7 +54,6 @@ extern void async_enable_stdin (void);
 
 extern int async_command_editing_p;
 extern int exec_done_display_p;
-extern char *async_annotation_suffix;
 extern struct prompts the_prompts;
 extern void (*call_readline) (void *);
 extern void (*input_handler) (char *);
diff --git a/gdb/top.c b/gdb/top.c
index b5ee4af..fb1657a 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1897,10 +1897,6 @@ init_main (void)
      the DEFAULT_PROMPT is.  */
   set_prompt (DEFAULT_PROMPT);
 
-  /* Set things up for annotation_level > 1, if the user ever decides
-     to use it.  */
-  async_annotation_suffix = "prompt";
-
   /* Set the important stuff up for command editing.  */
   command_editing_p = 1;
   history_expansion_p = 0;
-- 
1.9.3

Re: [PATCH 00/10] Command line input handling TLC

[Sergio Durigan Junior]

On Thursday, February 18 2016, Pedro Alves wrote:

> I looked a bit at changing immediate_quit users in order to get rid of
> throws from signal handlers, for the C++ conversion, and ended up
> stumbling on gdb's input code...
>
> So today I split this patch:
>
>   [PATCH 02/23] Command line input handling TLC
>   https://sourceware.org/ml/gdb-patches/2016-02/msg00070.html
>
> into smaller pieces.  I think this can go in independently of the rest
> of that series.

Thanks, Pedro.

I looked at all the patches (though I admit that I did not spend a lot
of time on the last patch), and they all seemed mostly fine by me.  When
I had something to say about a specific patch, I sent a comment.

IMHO, this series is basically ready to go in indeed.

Thanks for doing this.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 00/10] Command line input handling TLC

[Simon Marchi]

On 16-02-18 05:57 PM, Sergio Durigan Junior wrote:
> Thanks, Pedro.
> 
> I looked at all the patches (though I admit that I did not spend a lot
> of time on the last patch), and they all seemed mostly fine by me.  When
> I had something to say about a specific patch, I sent a comment.
> 
> IMHO, this series is basically ready to go in indeed.
> 
> Thanks for doing this.

I also gave it a look, it looks fine to me.  I don't understand everything, but
there are less things I don't understand in the new version than in the old
version, so it must be a good sign!

Simon

Re: [PATCH 00/10] Command line input handling TLC

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> I looked a bit at changing immediate_quit users in order to get rid of
> throws from signal handlers, for the C++ conversion, and ended up
> stumbling on gdb's input code...
>
> So today I split this patch:
>
>   [PATCH 02/23] Command line input handling TLC
>   https://sourceware.org/ml/gdb-patches/2016-02/msg00070.html
>
> into smaller pieces.  I think this can go in independently of the rest
> of that series.

Thanks for splitting patches...

>
> The original motivation here was factor out all the globals used by
> the command line input handling code to a structure, so that we can
> keep multiple instances of that later on.  But, I found that this code
> is a lot more messier than it needs to be, thus this series cleans
> things up significantly in the process as well.
>
> I tried to split the last patch further into smaller independent
> pieces, but failed...

These patches look good to me.

-- 
Yao (齐尧)

Re: [PATCH 00/10] Command line input handling TLC

[Pedro Alves]

Thanks everyone for the reviews.  I've pushed this in.

Thanks,
Pedro Alves