[PATCH] TUI: rewrite tui_query_hook()

[PATCH] TUI: rewrite tui_query_hook()

[Patrick Palka]

This patch rewrites tui_query_hook() to print things via tui_puts() and
to read in a line of input via wgetnstr().  The main motivation for this
rewrite is to get the backspace key to work correctly during a quit
prompt so that the user can revise their answer before pressing enter.
The backspace key now works correctly because we now use getstr()
instead of successive calls to getch().

gdb/ChangeLog:

	* tui/tui-hooks.c (tui_query_hook): Rewrite to use tui_puts and
	wgetnstr.
---
 gdb/tui/tui-hooks.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 6ba6285..9dee840 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -68,9 +68,9 @@ tui_query_hook (const char *msg, va_list argp)
 {
   int retval;
   int ans2;
-  int answer;
   char *question;
   struct cleanup *old_chain;
+  WINDOW *win = TUI_CMD_WIN->generic.handle;
 
   /* Format the question outside of the loop, to avoid reusing
      ARGP.  */
@@ -80,30 +80,18 @@ tui_query_hook (const char *msg, va_list argp)
   echo ();
   while (1)
     {
-      wrap_here ("");		/* Flush any buffered output.  */
-      gdb_flush (gdb_stdout);
+      char response[2], answer;
 
-      fputs_filtered (question, gdb_stdout);
-      printf_filtered (_("(y or n) "));
+      tui_puts (question);
+      tui_puts (_("(y or n) "));
 
-      wrap_here ("");
-      gdb_flush (gdb_stdout);
-
-      answer = tui_getc (stdin);
-      clearerr (stdin);		/* in case of C-d */
-      if (answer == EOF)	/* C-d */
+      if (wgetnstr (win, response, 1) == ERR)
 	{
 	  retval = 1;
 	  break;
 	}
-      /* Eat rest of input line, to EOF or newline.  */
-      if (answer != '\n')
-	do
-	  {
-            ans2 = tui_getc (stdin);
-	    clearerr (stdin);
-	  }
-	while (ans2 != EOF && ans2 != '\n' && ans2 != '\r');
+
+      answer = response[0];
 
       if (answer >= 'a')
 	answer -= 040;
@@ -117,10 +105,13 @@ tui_query_hook (const char *msg, va_list argp)
 	  retval = 0;
 	  break;
 	}
-      printf_filtered (_("Please answer y or n.\n"));
+      tui_puts (_("Please answer y or n.\n"));
     }
   noecho ();
 
+  /* Update our knowledge of the cursor position.  */
+  tui_puts ("");
+
   do_cleanups (old_chain);
   return retval;
 }
-- 
2.2.1.212.gc5b9256

Re: [PATCH] TUI: rewrite tui_query_hook()

[Pedro Alves]

On 01/08/2015 03:50 AM, Patrick Palka wrote:
> This patch rewrites tui_query_hook() to print things via tui_puts() and
> to read in a line of input via wgetnstr().  The main motivation for this
> rewrite is to get the backspace key to work correctly during a quit
> prompt so that the user can revise their answer before pressing enter.
> The backspace key now works correctly because we now use getstr()
> instead of successive calls to getch().

Pagination, done in prompt_for_continue, gets away without this hook, as
it's written in terms of readline.  If we did the same to defaulted_query,
I think the default code would just work for the TUI too.  Did you
consider that?

Thanks,
Pedro Alves

Re: [PATCH] TUI: rewrite tui_query_hook()

[Patrick Palka]

On Thu, Jan 8, 2015 at 6:03 AM, Pedro Alves <palves@redhat.com> wrote:
> On 01/08/2015 03:50 AM, Patrick Palka wrote:
>> This patch rewrites tui_query_hook() to print things via tui_puts() and
>> to read in a line of input via wgetnstr().  The main motivation for this
>> rewrite is to get the backspace key to work correctly during a quit
>> prompt so that the user can revise their answer before pressing enter.
>> The backspace key now works correctly because we now use getstr()
>> instead of successive calls to getch().
>
> Pagination, done in prompt_for_continue, gets away without this hook, as
> it's written in terms of readline.  If we did the same to defaulted_query,
> I think the default code would just work for the TUI too.  Did you
> consider that?

This is complicated by the fact that the default query code inserts
annotations before and after the query.  If we use
gdb_readline_wrapper to print the query and wait for input then the
2nd annotation will not be printed in a timely manner because
gdb_readline_wrapper blocks until a response is given by the user.

>
> Thanks,
> Pedro Alves
>

Re: [PATCH] TUI: rewrite tui_query_hook()

[Eli Zaretskii]

> From: Patrick Palka <patrick@parcs.ath.cx>
> Cc: Patrick Palka <patrick@parcs.ath.cx>
> Date: Wed,  7 Jan 2015 22:50:48 -0500
> 
> @@ -80,30 +80,18 @@ tui_query_hook (const char *msg, va_list argp)
>    echo ();
>    while (1)
>      {
> -      wrap_here ("");		/* Flush any buffered output.  */
> -      gdb_flush (gdb_stdout);
> +      char response[2], answer;
>  
> -      fputs_filtered (question, gdb_stdout);
> -      printf_filtered (_("(y or n) "));
> +      tui_puts (question);
> +      tui_puts (_("(y or n) "));
>  
> -      wrap_here ("");
> -      gdb_flush (gdb_stdout);
> -
> -      answer = tui_getc (stdin);
> -      clearerr (stdin);		/* in case of C-d */
> -      if (answer == EOF)	/* C-d */
> +      if (wgetnstr (win, response, 1) == ERR)

Given the latest discussions about buffering, don't you need to call
wrefresh after the second tui_puts?

Thanks.

Re: [PATCH] TUI: rewrite tui_query_hook()

[Patrick Palka]

On Thu, Jan 8, 2015 at 8:31 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Patrick Palka <patrick@parcs.ath.cx>
>> Cc: Patrick Palka <patrick@parcs.ath.cx>
>> Date: Wed,  7 Jan 2015 22:50:48 -0500
>>
>> @@ -80,30 +80,18 @@ tui_query_hook (const char *msg, va_list argp)
>>    echo ();
>>    while (1)
>>      {
>> -      wrap_here ("");                /* Flush any buffered output.  */
>> -      gdb_flush (gdb_stdout);
>> +      char response[2], answer;
>>
>> -      fputs_filtered (question, gdb_stdout);
>> -      printf_filtered (_("(y or n) "));
>> +      tui_puts (question);
>> +      tui_puts (_("(y or n) "));
>>
>> -      wrap_here ("");
>> -      gdb_flush (gdb_stdout);
>> -
>> -      answer = tui_getc (stdin);
>> -      clearerr (stdin);              /* in case of C-d */
>> -      if (answer == EOF)     /* C-d */
>> +      if (wgetnstr (win, response, 1) == ERR)
>
> Given the latest discussions about buffering, don't you need to call
> wrefresh after the second tui_puts?

I have been using this patch for a few months locally and have not
seen any buffering issues.  The recently-mentioned issues are mostly
Windows-related, aren't they?  I can add a wrefresh() if that is
what's needed to prevent potential buffering issues.

>
> Thanks.

Re: [PATCH] TUI: rewrite tui_query_hook()

[Pedro Alves]

On 01/08/2015 12:40 PM, Patrick Palka wrote:

> If we use
> gdb_readline_wrapper to print the query and wait for input then the
> 2nd annotation will not be printed in a timely manner because
> gdb_readline_wrapper blocks until a response is given by the user.

Can't we just cat the annotations bits into the query string
itself?  IOW, make them part of the secondary prompt.

Thanks,
Pedro Alves

Re: [PATCH] TUI: rewrite tui_query_hook()

[Pedro Alves]

On 01/08/2015 01:42 PM, Patrick Palka wrote:
> On Thu, Jan 8, 2015 at 8:31 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Patrick Palka <patrick@parcs.ath.cx>
>>> Cc: Patrick Palka <patrick@parcs.ath.cx>
>>> Date: Wed,  7 Jan 2015 22:50:48 -0500
>>>
>>> @@ -80,30 +80,18 @@ tui_query_hook (const char *msg, va_list argp)
>>>    echo ();
>>>    while (1)
>>>      {
>>> -      wrap_here ("");                /* Flush any buffered output.  */
>>> -      gdb_flush (gdb_stdout);
>>> +      char response[2], answer;
>>>
>>> -      fputs_filtered (question, gdb_stdout);
>>> -      printf_filtered (_("(y or n) "));
>>> +      tui_puts (question);
>>> +      tui_puts (_("(y or n) "));
>>>
>>> -      wrap_here ("");
>>> -      gdb_flush (gdb_stdout);
>>> -
>>> -      answer = tui_getc (stdin);
>>> -      clearerr (stdin);              /* in case of C-d */
>>> -      if (answer == EOF)     /* C-d */
>>> +      if (wgetnstr (win, response, 1) == ERR)
>>
>> Given the latest discussions about buffering, don't you need to call
>> wrefresh after the second tui_puts?
> 
> I have been using this patch for a few months locally and have not
> seen any buffering issues.  

That's because presently tui_puts always flushes.  You'll need it
since the query string doesn't end in a new line.  This is exactly
like  we have a gdb_flush in defaulted_query after printing the
query question.

> The recently-mentioned issues are mostly
> Windows-related, aren't they?  I can add a wrefresh() if that is
> what's needed to prevent potential buffering issues.

Let's leave that either for the buffering patch, or if that patch
goes in before this one, add the flushing call here then.  That patch
(or patches..) adds a function to handle the flushing, rather than
calling wrefresh, and you'd want to call that one.

Thanks,
Pedro Alves

Re: [PATCH] TUI: rewrite tui_query_hook()

[Patrick Palka]

On Thu, Jan 8, 2015 at 8:53 AM, Pedro Alves <palves@redhat.com> wrote:
> On 01/08/2015 12:40 PM, Patrick Palka wrote:
>
>> If we use
>> gdb_readline_wrapper to print the query and wait for input then the
>> 2nd annotation will not be printed in a timely manner because
>> gdb_readline_wrapper blocks until a response is given by the user.
>
> Can't we just cat the annotations bits into the query string
> itself?  IOW, make them part of the secondary prompt.

I'm not sure because the annotations contain newlines, so (with
annotations enabled) the prompt passed to readline would have
newlines.  I do not know if readline supports multi-line prompts.

But IMO a consolidation of the custom TUI query hook and the default
query hook is a quite separate endeavor.  I think it should be left as
future work.

>
> Thanks,
> Pedro Alves
>

Re: [PATCH] TUI: rewrite tui_query_hook()

[Eli Zaretskii]

> From: Patrick Palka <patrick@parcs.ath.cx>
> Date: Thu, 8 Jan 2015 08:42:38 -0500
> Cc: gdb-patches@sourceware.org
> 
> > Given the latest discussions about buffering, don't you need to call
> > wrefresh after the second tui_puts?
> 
> I have been using this patch for a few months locally and have not
> seen any buffering issues.  The recently-mentioned issues are mostly
> Windows-related, aren't they?  I can add a wrefresh() if that is
> what's needed to prevent potential buffering issues.

I think we shouldn't assume that stdout gets flushed when a program
reads from stdin.

Re: [PATCH] TUI: rewrite tui_query_hook()

[Patrick Palka]

On Thu, Jan 8, 2015 at 9:10 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Thu, Jan 8, 2015 at 8:53 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 01/08/2015 12:40 PM, Patrick Palka wrote:
>>
>>> If we use
>>> gdb_readline_wrapper to print the query and wait for input then the
>>> 2nd annotation will not be printed in a timely manner because
>>> gdb_readline_wrapper blocks until a response is given by the user.
>>
>> Can't we just cat the annotations bits into the query string
>> itself?  IOW, make them part of the secondary prompt.
>
> I'm not sure because the annotations contain newlines, so (with
> annotations enabled) the prompt passed to readline would have
> newlines.  I do not know if readline supports multi-line prompts.
>
> But IMO a consolidation of the custom TUI query hook and the default
> query hook is a quite separate endeavor.  I think it should be left as
> future work.

And in the meantime this patch takes advantage of the existence of a
custom TUI query hook to at least make queries within TUI to behave
nicely.

>
>>
>> Thanks,
>> Pedro Alves
>>

Re: [PATCH] TUI: rewrite tui_query_hook()

[Patrick Palka]

On Thu, Jan 8, 2015 at 9:10 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Thu, Jan 8, 2015 at 8:53 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 01/08/2015 12:40 PM, Patrick Palka wrote:
>>
>>> If we use
>>> gdb_readline_wrapper to print the query and wait for input then the
>>> 2nd annotation will not be printed in a timely manner because
>>> gdb_readline_wrapper blocks until a response is given by the user.
>>
>> Can't we just cat the annotations bits into the query string
>> itself?  IOW, make them part of the secondary prompt.
>
> I'm not sure because the annotations contain newlines, so (with
> annotations enabled) the prompt passed to readline would have
> newlines.  I do not know if readline supports multi-line prompts.

Actually readline does support multi-line prompts because what you
mentioned is exactly what prompt_for_continue() does.  It concatenates
the annotations to the prompt string and passes the multi-line prompt
to gdb_readline_wrapper().  So it will be easy to consolidate these
two query hooks.  I will try to do so.

>
> But IMO a consolidation of the custom TUI query hook and the default
> query hook is a quite separate endeavor.  I think it should be left as
> future work.
>
>>
>> Thanks,
>> Pedro Alves
>>

Re: [PATCH] TUI: rewrite tui_query_hook()

[Patrick Palka]

On Thu, Jan 8, 2015 at 9:14 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Patrick Palka <patrick@parcs.ath.cx>
>> Date: Thu, 8 Jan 2015 08:42:38 -0500
>> Cc: gdb-patches@sourceware.org
>>
>> > Given the latest discussions about buffering, don't you need to call
>> > wrefresh after the second tui_puts?
>>
>> I have been using this patch for a few months locally and have not
>> seen any buffering issues.  The recently-mentioned issues are mostly
>> Windows-related, aren't they?  I can add a wrefresh() if that is
>> what's needed to prevent potential buffering issues.
>
> I think we shouldn't assume that stdout gets flushed when a program
> reads from stdin.

That sounds like a good rule of thumb.

[PATCH] Consolidate the custom TUI query hook with default query hook

[Patrick Palka]

Hi Pedro,

This is what I have so far.  It seems to work well.  Thoughts?

-- >8 --

This patch primarily rewrites defaulted_query() to use
gdb_readline_wrapper() to prompt the user for input, like
prompt_for_continue() does.  The motivation for this rewrite is to be
able to reuse the default query hook in TUI, obviating the need for a
custom TUI query hook.

gdb/ChangeLog:

	* utils.c (defaulted_query): Rewrite to use gdb_readline_wrapper
	to prompt for input.
	* tui/tui-hooks.c (tui_query_hook): Remove.
	(tui_install_hooks): Don't set deprecated_query_hook.
---
 gdb/tui/tui-hooks.c | 64 -----------------------------------------------------
 gdb/utils.c         | 56 ++++++++++------------------------------------
 2 files changed, 12 insertions(+), 108 deletions(-)

diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 6ba6285..e53f526 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -63,68 +63,6 @@ tui_new_objfile_hook (struct objfile* objfile)
     tui_display_main ();
 }
 
-static int ATTRIBUTE_PRINTF (1, 0)
-tui_query_hook (const char *msg, va_list argp)
-{
-  int retval;
-  int ans2;
-  int answer;
-  char *question;
-  struct cleanup *old_chain;
-
-  /* Format the question outside of the loop, to avoid reusing
-     ARGP.  */
-  question = xstrvprintf (msg, argp);
-  old_chain = make_cleanup (xfree, question);
-
-  echo ();
-  while (1)
-    {
-      wrap_here ("");		/* Flush any buffered output.  */
-      gdb_flush (gdb_stdout);
-
-      fputs_filtered (question, gdb_stdout);
-      printf_filtered (_("(y or n) "));
-
-      wrap_here ("");
-      gdb_flush (gdb_stdout);
-
-      answer = tui_getc (stdin);
-      clearerr (stdin);		/* in case of C-d */
-      if (answer == EOF)	/* C-d */
-	{
-	  retval = 1;
-	  break;
-	}
-      /* Eat rest of input line, to EOF or newline.  */
-      if (answer != '\n')
-	do
-	  {
-            ans2 = tui_getc (stdin);
-	    clearerr (stdin);
-	  }
-	while (ans2 != EOF && ans2 != '\n' && ans2 != '\r');
-
-      if (answer >= 'a')
-	answer -= 040;
-      if (answer == 'Y')
-	{
-	  retval = 1;
-	  break;
-	}
-      if (answer == 'N')
-	{
-	  retval = 0;
-	  break;
-	}
-      printf_filtered (_("Please answer y or n.\n"));
-    }
-  noecho ();
-
-  do_cleanups (old_chain);
-  return retval;
-}
-
 /* Prevent recursion of deprecated_register_changed_hook().  */
 static int tui_refreshing_registers = 0;
 
@@ -263,8 +201,6 @@ tui_install_hooks (void)
   deprecated_print_frame_info_listing_hook
     = tui_print_frame_info_listing_hook;
 
-  deprecated_query_hook = tui_query_hook;
-
   /* Install the event hooks.  */
   tui_bp_created_observer
     = observer_attach_breakpoint_created (tui_event_create_breakpoint);
diff --git a/gdb/utils.c b/gdb/utils.c
index 084db87..187f91a 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1198,12 +1198,11 @@ compile_rx_or_error (regex_t *pattern, const char *rx, const char *message)
 static int ATTRIBUTE_PRINTF (1, 0)
 defaulted_query (const char *ctlstr, const char defchar, va_list args)
 {
-  int answer;
   int ans2;
   int retval;
   int def_value;
   char def_answer, not_def_answer;
-  char *y_string, *n_string, *question;
+  char *y_string, *n_string, *question, *prompt;
   /* Used to add duration we waited for user to respond to
      prompt_for_continue_wait_time.  */
   struct timeval prompt_started, prompt_ended, prompt_delta;
@@ -1263,62 +1262,31 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
 
   /* Format the question outside of the loop, to avoid reusing args.  */
   question = xstrvprintf (ctlstr, args);
+  prompt = xstrprintf ("%s%s(%s or %s) %s",
+		      annotation_level > 1 ? "\n\032\032pre-query\n" : "",
+		      question, y_string, n_string,
+		      annotation_level > 1 ? "\n\032\032query\n" : "");
+  xfree (question);
 
   /* Used for calculating time spend waiting for user.  */
   gettimeofday (&prompt_started, NULL);
 
   while (1)
     {
-      wrap_here ("");		/* Flush any buffered output.  */
-      gdb_flush (gdb_stdout);
-
-      if (annotation_level > 1)
-	printf_filtered (("\n\032\032pre-query\n"));
-
-      fputs_filtered (question, gdb_stdout);
-      printf_filtered (_("(%s or %s) "), y_string, n_string);
-
-      if (annotation_level > 1)
-	printf_filtered (("\n\032\032query\n"));
+      char *response, answer = EOF;
 
-      wrap_here ("");
       gdb_flush (gdb_stdout);
+      response = gdb_readline_wrapper (prompt);
+      if (response != NULL)
+	answer = response[0];
+      xfree (response);
 
-      answer = fgetc (stdin);
-
-      /* We expect fgetc to block until a character is read.  But
-         this may not be the case if the terminal was opened with
-         the NONBLOCK flag.  In that case, if there is nothing to
-         read on stdin, fgetc returns EOF, but also sets the error
-         condition flag on stdin and errno to EAGAIN.  With a true
-         EOF, stdin's error condition flag is not set.
-
-         A situation where this behavior was observed is a pseudo
-         terminal on AIX.  */
-      while (answer == EOF && ferror (stdin) && errno == EAGAIN)
-        {
-          /* Not a real EOF.  Wait a little while and try again until
-             we read something.  */
-          clearerr (stdin);
-          gdb_usleep (10000);
-          answer = fgetc (stdin);
-        }
-
-      clearerr (stdin);		/* in case of C-d */
       if (answer == EOF)	/* C-d */
 	{
 	  printf_filtered ("EOF [assumed %c]\n", def_answer);
 	  retval = def_value;
 	  break;
 	}
-      /* Eat rest of input line, to EOF or newline.  */
-      if (answer != '\n')
-	do
-	  {
-	    ans2 = fgetc (stdin);
-	    clearerr (stdin);
-	  }
-	while (ans2 != EOF && ans2 != '\n' && ans2 != '\r');
 
       if (answer >= 'a')
 	answer -= 040;
@@ -1350,7 +1318,7 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
   timeval_add (&prompt_for_continue_wait_time,
                &prompt_for_continue_wait_time, &prompt_delta);
 
-  xfree (question);
+  xfree (prompt);
   if (annotation_level > 1)
     printf_filtered (("\n\032\032post-query\n"));
   return retval;
-- 
2.2.1.212.gc5b9256

Re: [PATCH] Consolidate the custom TUI query hook with default query hook

[Pedro Alves]

On 01/08/2015 03:17 PM, Patrick Palka wrote:
> Hi Pedro,
> 
> This is what I have so far.  It seems to work well.  Thoughts?

Nice!

> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1198,12 +1198,11 @@ compile_rx_or_error (regex_t *pattern, const char *rx, const char *message)
>  static int ATTRIBUTE_PRINTF (1, 0)
>  defaulted_query (const char *ctlstr, const char defchar, va_list args)
>  {
> -  int answer;
>    int ans2;
>    int retval;
>    int def_value;
>    char def_answer, not_def_answer;
> -  char *y_string, *n_string, *question;
> +  char *y_string, *n_string, *question, *prompt;
>    /* Used to add duration we waited for user to respond to
>       prompt_for_continue_wait_time.  */
>    struct timeval prompt_started, prompt_ended, prompt_delta;
> @@ -1263,62 +1262,31 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
>  
>    /* Format the question outside of the loop, to avoid reusing args.  */
>    question = xstrvprintf (ctlstr, args);
> +  prompt = xstrprintf ("%s%s(%s or %s) %s",

Add _() here, for the "or".

> -      printf_filtered (_("(%s or %s) "), y_string, n_string);

Notice here we had it.

> -
> -      if (annotation_level > 1)
> -	printf_filtered (("\n\032\032query\n"));
> +      char *response, answer = EOF;
>  
> -      wrap_here ("");
>        gdb_flush (gdb_stdout);
> +      response = gdb_readline_wrapper (prompt);
> +      if (response != NULL)
> +	answer = response[0];
> +      xfree (response);
>  
> -      answer = fgetc (stdin);
> -
> -      /* We expect fgetc to block until a character is read.  But
> -         this may not be the case if the terminal was opened with
> -         the NONBLOCK flag.  In that case, if there is nothing to
> -         read on stdin, fgetc returns EOF, but also sets the error
> -         condition flag on stdin and errno to EAGAIN.  With a true
> -         EOF, stdin's error condition flag is not set.
> -
> -         A situation where this behavior was observed is a pseudo
> -         terminal on AIX.  */
> -      while (answer == EOF && ferror (stdin) && errno == EAGAIN)
> -        {
> -          /* Not a real EOF.  Wait a little while and try again until
> -             we read something.  */
> -          clearerr (stdin);
> -          gdb_usleep (10000);
> -          answer = fgetc (stdin);
> -        }
> -
> -      clearerr (stdin);		/* in case of C-d */
>        if (answer == EOF)	/* C-d */
>  	{
>  	  printf_filtered ("EOF [assumed %c]\n", def_answer);
>  	  retval = def_value;
>  	  break;
>  	}

Can you move this a bit above, and write the check in
terms of response, like this?

        if (response == NULL)	/* C-d */
  	{
  	  printf_filtered ("EOF [assumed %c]\n", def_answer);
  	  retval = def_value;
  	  break;
  	}

I think that's a little bit clearer.  What got me to that was that
you made "answer" a char (from int), which may be signed or unsigned,
depending on host, and then the >= 'a' check further below looked
suspicious, though I see that it's unreachable for the EOF case.  I think
that way there'll no be reference to EOF at all in the
code (except in the printf string).

Other than that, if this passes testing, that it's OK with me.

Thanks for doing this.

-- 
Pedro Alves