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

[Pedro Alves <palves@redhat.com>]

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