PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[H. J. Lu]

The problem is callback in readline 5.1 is changed. When gdb readline
callback calls readline (), readline is really confused since although
it is called from gdb callback, it isn't really in callback state. This
kludge seems to work for me.


H.J.
----
2006-11-21  H.J. Lu  <hongjiu.lu@intel.com>

	PR tui/2173
	* top.c (gdb_readline_wrapper): Unset and reset RL_STATE_CALLBACK
	around readline if needed.

--- gdb/top.c.arrow	2006-07-21 07:46:53.000000000 -0700
+++ gdb/top.c	2006-11-21 13:20:04.000000000 -0800
@@ -724,6 +724,9 @@ The filename in which to record the comm
 char *
 gdb_readline_wrapper (char *prompt)
 {
+  char *line;
+  int in_callback;
+
   /* Set the hook that works in this case.  */
   if (after_char_processing_hook)
     {
@@ -731,7 +734,19 @@ gdb_readline_wrapper (char *prompt)
       after_char_processing_hook = NULL;
     }
 
-  return readline (prompt);
+  /* When we call readline, we have to make sure that readline isn't in
+     the callback state. Otherwise, it will get really confused.
+     PR tui/2173.  */
+  in_callback = RL_ISSTATE (RL_STATE_CALLBACK);
+  if (in_callback)
+    RL_UNSETSTATE (RL_STATE_CALLBACK);
+
+  line = readline (prompt);
+
+  if (in_callback)
+    RL_SETSTATE (RL_STATE_CALLBACK);
+
+  return line;
 }
 
 \f

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Tue, Nov 21, 2006 at 01:32:05PM -0800, H. J. Lu wrote:
> The problem is callback in readline 5.1 is changed. When gdb readline
> callback calls readline (), readline is really confused since although
> it is called from gdb callback, it isn't really in callback state. This
> kludge seems to work for me.

I'm pretty sure this isn't right.  I got as far as figuring out that we
should be calling rl_callback_handler_install and
rl_callback_handler_remove at different times, always removing the
handler before calling readline recursively, but I couldn't quite work
out the right conditions.

What would be really best would be to stop calling readline directly;
make the whole thing involve the event loop, just like top level
command line editing.  That's going to be a bit of work though.


-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[H. J. Lu]

On Tue, Nov 28, 2006 at 11:46:58AM -0500, Daniel Jacobowitz wrote:
> On Tue, Nov 21, 2006 at 01:32:05PM -0800, H. J. Lu wrote:
> > The problem is callback in readline 5.1 is changed. When gdb readline
> > callback calls readline (), readline is really confused since although
> > it is called from gdb callback, it isn't really in callback state. This
> > kludge seems to work for me.
> 
> I'm pretty sure this isn't right.  I got as far as figuring out that we
> should be calling rl_callback_handler_install and
> rl_callback_handler_remove at different times, always removing the
> handler before calling readline recursively, but I couldn't quite work
> out the right conditions.

I assume by "this isn't right",  you mean my patch may break something.
Do you have a testcase? It may get into readline:

http://lists.gnu.org/archive/html/bug-readline/2006-11/msg00011.html


H.J.

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Tue, Nov 28, 2006 at 08:58:44AM -0800, H. J. Lu wrote:
> On Tue, Nov 28, 2006 at 11:46:58AM -0500, Daniel Jacobowitz wrote:
> > On Tue, Nov 21, 2006 at 01:32:05PM -0800, H. J. Lu wrote:
> > > The problem is callback in readline 5.1 is changed. When gdb readline
> > > callback calls readline (), readline is really confused since although
> > > it is called from gdb callback, it isn't really in callback state. This
> > > kludge seems to work for me.
> > 
> > I'm pretty sure this isn't right.  I got as far as figuring out that we
> > should be calling rl_callback_handler_install and
> > rl_callback_handler_remove at different times, always removing the
> > handler before calling readline recursively, but I couldn't quite work
> > out the right conditions.
> 
> I assume by "this isn't right",  you mean my patch may break something.
> Do you have a testcase? It may get into readline:
> 
> http://lists.gnu.org/archive/html/bug-readline/2006-11/msg00011.html

I don't think it's right because it's messing around with the internals
of readline from GDB.  If it's applied to readline that's an entirely
different story; then it looks like a fine fix.

With the version Jan sent to bug-readline, I worry that readline and GDB
will get confused if we longjmp out of the readline handler; you can
make that happen by typing:

(gdb) define foo
> <Control-C>

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[H. J. Lu]

On Tue, Nov 28, 2006 at 08:58:44AM -0800, H. J. Lu wrote:
> On Tue, Nov 28, 2006 at 11:46:58AM -0500, Daniel Jacobowitz wrote:
> > On Tue, Nov 21, 2006 at 01:32:05PM -0800, H. J. Lu wrote:
> > > The problem is callback in readline 5.1 is changed. When gdb readline
> > > callback calls readline (), readline is really confused since although
> > > it is called from gdb callback, it isn't really in callback state. This
> > > kludge seems to work for me.
> > 
> > I'm pretty sure this isn't right.  I got as far as figuring out that we
> > should be calling rl_callback_handler_install and
> > rl_callback_handler_remove at different times, always removing the
> > handler before calling readline recursively, but I couldn't quite work
> > out the right conditions.
> 
> I assume by "this isn't right",  you mean my patch may break something.
> Do you have a testcase? It may get into readline:
> 
> http://lists.gnu.org/archive/html/bug-readline/2006-11/msg00011.html
> 
> 

Here is the updated patch for readline.

(gdb) define foo
> <Control-C>

works with it.


H.J.
----
2006-12-02  H.J. Lu  <hongjiu.lu@intel.com>
	    Jan Kratochvil <jkratoch@redhat.com>

	PR tui/2173
	* readline.c (readline): Unset and reset RL_STATE_CALLBACK if
	needed.

Index: readline/readline.c
===================================================================
RCS file: /cvs/src/src/readline/readline.c,v
retrieving revision 1.10
diff -u -p -r1.10 readline.c
--- readline/readline.c	5 May 2006 18:26:12 -0000	1.10
+++ readline/readline.c	22 Nov 2006 19:40:17 -0000
@@ -295,6 +295,7 @@ readline (prompt)
      const char *prompt;
 {
   char *value;
+  int in_callback;
 
   /* If we are at EOF return a NULL string. */
   if (rl_pending_input == EOF)
@@ -303,6 +304,13 @@ readline (prompt)
       return ((char *)NULL);
     }
 
+  /* When we call readline, we have to make sure that readline isn't in
+     the callback state. Otherwise, it will get really confused.
+     PR gdb tui/2173.  */
+  in_callback = RL_ISSTATE (RL_STATE_CALLBACK);
+  if (in_callback)
+    RL_UNSETSTATE (RL_STATE_CALLBACK);
+
   rl_set_prompt (prompt);
 
   rl_initialize ();
@@ -321,6 +329,9 @@ readline (prompt)
   rl_clear_signals ();
 #endif
 
+  if (in_callback)
+    RL_SETSTATE (RL_STATE_CALLBACK);
+
   return (value);
 }
 

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Sat, Dec 02, 2006 at 10:43:44AM -0800, H. J. Lu wrote:
> 2006-12-02  H.J. Lu  <hongjiu.lu@intel.com>
> 	    Jan Kratochvil <jkratoch@redhat.com>
> 
> 	PR tui/2173
> 	* readline.c (readline): Unset and reset RL_STATE_CALLBACK if
> 	needed.

This looks reasonable to me; I was wrong about longjmp since Readline's
signal handlers will be installed.  If Chet accepts this or a similar
fix for the next release of readline, then you can apply it to our
copy.


-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Joel Brobecker]

> > 	PR tui/2173
> > 	* readline.c (readline): Unset and reset RL_STATE_CALLBACK if
> > 	needed.
> 
> This looks reasonable to me; I was wrong about longjmp since Readline's
> signal handlers will be installed.  If Chet accepts this or a similar
> fix for the next release of readline, then you can apply it to our
> copy.

Assuming that Chet takes it for readline, do you think we can/should
put this in the branch as well?

Thanks,
-- 
Joel

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Chet Ramey]

Daniel Jacobowitz wrote:
> On Sat, Dec 02, 2006 at 10:43:44AM -0800, H. J. Lu wrote:
>> 2006-12-02  H.J. Lu  <hongjiu.lu@intel.com>
>> 	    Jan Kratochvil <jkratoch@redhat.com>
>>
>> 	PR tui/2173
>> 	* readline.c (readline): Unset and reset RL_STATE_CALLBACK if
>> 	needed.
> 
> This looks reasonable to me; I was wrong about longjmp since Readline's
> signal handlers will be installed.  If Chet accepts this or a similar
> fix for the next release of readline, then you can apply it to our
> copy.

In general, readline's signal handlers will restore any application-
installed handlers before resending the signal to itself.  This means
that any signal can potentially cause an application's handler to be
called, and longjmp to be called from that.

Now we would be relying on the application's signal handler to restore
the appropriate readline state.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		       Live Strong.  No day but today.
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Chet Ramey]

H. J. Lu wrote:

>>> I'm pretty sure this isn't right.  I got as far as figuring out that we
>>> should be calling rl_callback_handler_install and
>>> rl_callback_handler_remove at different times, always removing the
>>> handler before calling readline recursively, but I couldn't quite work
>>> out the right conditions.
>> I assume by "this isn't right",  you mean my patch may break something.
>> Do you have a testcase? It may get into readline:

Unless the calling application is careful, this code will leave readline
in an inconsistent state in the presence of a longjmp().  It relies on
private state kept local to a single call to readline().

I am leaning towards not including it for that reason.

There should be no reason that the application cannot remove the callback
handler and re-add it before calling readline synchronously, as Daniel or
H.J. suggested.  The application is the only one in a position to know
which is right.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		       Live Strong.  No day but today.
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[H. J. Lu]

On Sat, Dec 02, 2006 at 02:08:26PM -0500, Chet Ramey wrote:
> H. J. Lu wrote:
> 
> >>> I'm pretty sure this isn't right.  I got as far as figuring out that we
> >>> should be calling rl_callback_handler_install and
> >>> rl_callback_handler_remove at different times, always removing the
> >>> handler before calling readline recursively, but I couldn't quite work
> >>> out the right conditions.
> >> I assume by "this isn't right",  you mean my patch may break something.
> >> Do you have a testcase? It may get into readline:
> 
> Unless the calling application is careful, this code will leave readline
> in an inconsistent state in the presence of a longjmp().  It relies on
> private state kept local to a single call to readline().
> 
> I am leaning towards not including it for that reason.
> 
> There should be no reason that the application cannot remove the callback
> handler and re-add it before calling readline synchronously, as Daniel or
> H.J. suggested.  The application is the only one in a position to know
> which is right.
> 

Did you mean

http://sources.redhat.com/ml/gdb-patches/2006-11/msg00234.html

is more appropriate?


H.J.

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Sat, Dec 02, 2006 at 02:15:41PM -0800, H. J. Lu wrote:
> > There should be no reason that the application cannot remove the callback
> > handler and re-add it before calling readline synchronously, as Daniel or
> > H.J. suggested.  The application is the only one in a position to know
> > which is right.
> > 
> 
> Did you mean
> 
> http://sources.redhat.com/ml/gdb-patches/2006-11/msg00234.html
> 
> is more appropriate?

HJ, did you even read my response?

Messing about with RL_SETSTATE and RL_UNSETSTATE in GDB is not wise and
we should not be doing it.  We should be manually removing our
callbacks before we call readline synchronously, as I wrote in reply to
your message.  Someone is going to need to really understand the
different paths that take us into readline to work out where to do it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Chet Ramey]

H. J. Lu wrote:
> On Sat, Dec 02, 2006 at 02:08:26PM -0500, Chet Ramey wrote:
>> H. J. Lu wrote:
>>
>>>>> I'm pretty sure this isn't right.  I got as far as figuring out that we
>>>>> should be calling rl_callback_handler_install and
>>>>> rl_callback_handler_remove at different times, always removing the
>>>>> handler before calling readline recursively, but I couldn't quite work
>>>>> out the right conditions.
>>>> I assume by "this isn't right",  you mean my patch may break something.
>>>> Do you have a testcase? It may get into readline:
>> Unless the calling application is careful, this code will leave readline
>> in an inconsistent state in the presence of a longjmp().  It relies on
>> private state kept local to a single call to readline().
>>
>> I am leaning towards not including it for that reason.
>>
>> There should be no reason that the application cannot remove the callback
>> handler and re-add it before calling readline synchronously, as Daniel or
>> H.J. suggested.  The application is the only one in a position to know
>> which is right.
>>
> 
> Did you mean
> 
> http://sources.redhat.com/ml/gdb-patches/2006-11/msg00234.html
> 
> is more appropriate?

More appropriate in the sense that the application controls the state
and switches between the callback and synchronous readline modes.
Since it's the app that's supposed to be calling into readline when in
callback mode anyway, I think this patch will work better.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		       Live Strong.  No day but today.
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Jan Kratochvil]

On Sun, 03 Dec 2006 06:25:29 +0100, Chet Ramey wrote:
...
> More appropriate in the sense that the application controls the state
> and switches between the callback and synchronous readline modes.
> Since it's the app that's supposed to be calling into readline when in
> callback mode anyway, I think this patch will work better.

Chet, is it right it is more a design than implementation problem of readline?
readline cannot determine how many nested readline calls have been abandoned by
signal handler's longjmp ().  Therefore it cannot determine if the current mode
(the last unabandoned call) is a callback or synchronous one.

The sample code should be readline documentation compliant, still the called
function `_rl_next_macro_key' has undeterministic value of
`RL_ISSTATE (RL_STATE_CALLBACK)'.

Also any longjmp () from inside a signal handler is too dangerous as the data
structures are not locked against signals.  The signal handler should only set
some flag.  And the synchronous readline () function should be never used if
one needs to quit the input mode by SIGINT (as one cannot abort readline () if
not using the dangerous longjmp ()).

IMO the right way is to stop using longjmp from GDB's signal handlers.
Therefore to always use callbacked readline and stop the loop if detected
a flag set by the installed signal handler.


Thanks for info,
Jan

------------------------------------------------------------------------------

main ()
{
	if (setjmp (buf_A))
		{
			rl_read_key ();	/* -> _rl_next_macro_key () */
		}
	signal (SIGINT, handle_SIGINT_at_A);
	readline("mode-A");
	...
}
handle_SIGINT_at_A ()
{
	if (setjmp (buf_B))
		{
			rl_read_key ();	/* -> _rl_next_macro_key () */
		}
	signal (SIGINT, handle_SIGINT_at_B);
	rl_callback_handler_install ("mode-B", mode_B_command);
	for (;;)
		rl_callback_read_char ();
}
handle_SIGINT_at_B ()
{
	if (rand () & 1)
		longjmp (buf_A);
	else
		longjmp (buf_B);
}

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Chet Ramey]

Jan Kratochvil wrote:
> On Sun, 03 Dec 2006 06:25:29 +0100, Chet Ramey wrote:
> ...
>> More appropriate in the sense that the application controls the state
>> and switches between the callback and synchronous readline modes.
>> Since it's the app that's supposed to be calling into readline when in
>> callback mode anyway, I think this patch will work better.
> 
> Chet, is it right it is more a design than implementation problem of readline?
> readline cannot determine how many nested readline calls have been abandoned by
> signal handler's longjmp ().  Therefore it cannot determine if the current mode
> (the last unabandoned call) is a callback or synchronous one.

Not exactly.  The callback and traditional calling mechanisms are
orthogonal.  It's just not safe to mix them.  The longjmp leaves
readline in an inconsistent state, unless the calling application is
careful to undo what state-setting it has done.  That's the part
readline can't know about:  the appropriate application-specific part.

> 
> The sample code should be readline documentation compliant, still the called
> function `_rl_next_macro_key' has undeterministic value of
> `RL_ISSTATE (RL_STATE_CALLBACK)'.

OK, you got me.  I'll fix that one, if I can find it.  I can't find a call
to that function in the examples/ or doc/ directories.

> Also any longjmp () from inside a signal handler is too dangerous as the data
> structures are not locked against signals.  The signal handler should only set
> some flag.  And the synchronous readline () function should be never used if
> one needs to quit the input mode by SIGINT (as one cannot abort readline () if
> not using the dangerous longjmp ()).

Sure, in theory.  Pragmatically, however, it's not that bad.  We're just
talking about not mixing the two calling modes, since readline alters its
behavior (and makes certain assumptions) based on the current mode.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		       Live Strong.  No day but today.
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Jan Kratochvil]

Hi Chet,


On Mon, 18 Dec 2006 21:09:19 +0100, Chet Ramey wrote:
...
> Not exactly.  The callback and traditional calling mechanisms are
> orthogonal.  It's just not safe to mix them.  The longjmp leaves
> readline in an inconsistent state, unless the calling application is
> careful to undo what state-setting it has done.  That's the part
> readline can't know about:  the appropriate application-specific part.

OK, so going to provide callback based GDB reimplementation of
gdb_readline_wrapper () (readline () call).


> > The sample code should be readline documentation compliant, still the called
> > function `_rl_next_macro_key' has undeterministic value of
> > `RL_ISSTATE (RL_STATE_CALLBACK)'.
> 
> OK, you got me.  I'll fix that one, if I can find it.  I can't find a call
> to that function in the examples/ or doc/ directories.

rl_read_key () calls _rl_next_macro_key () and rl_read_key () can be called by
the application.  I did not check how much serious is a fail of this mode check
but I assume there are other functions callable by the application this way.


Regards,
Jan

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On Wed, 20 Dec 2006 00:19:26 +0100, Jan Kratochvil wrote:
> On Mon, 18 Dec 2006 21:09:19 +0100, Chet Ramey wrote:
> ...
> > Not exactly.  The callback and traditional calling mechanisms are
> > orthogonal.  It's just not safe to mix them.  The longjmp leaves
> > readline in an inconsistent state, unless the calling application is
> > careful to undo what state-setting it has done.  That's the part
> > readline can't know about:  the appropriate application-specific part.
> 
> OK, so going to provide callback based GDB reimplementation of
> gdb_readline_wrapper () (readline () call).

Here is one which forks for me but still it is more complicated than I expected.

I believe there may be side effects but unfortunately it can be expected the
interactive readline(3) usage is not well covered by the testsuite.
readline(3) call does a lot of various initializations currently omitted by the
callback-based gdb reimplementation.


Regards,
Jan

[-- Attachment #2: gdb-readline.patch --]
[-- Type: text/plain, Size: 6767 bytes --]

2006-12-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb/Makefile.in (top.o): Updated dependencies.
	* gdb/top.c: Include "event-loop.h"
	(gdb_readline_wrapper_result): New variable.
	(gdb_readline_wrapper_line): New function.
	(gdb_readline_wrapper_cleanup): New struct.
	(gdb_readline_wrapper_cleanup): New function.
	(gdb_readline_wrapper): Replace `readline' by a `gdb_do_one_event loop'.

2006-12-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/readline-callback-history.c,
	gdb.base/readline-callback-history.exp: New files.

--- gdb/Makefile.in	17 Dec 2006 13:30:43 -0000	1.861
+++ gdb/Makefile.in	25 Dec 2006 22:46:41 -0000
@@ -2792,7 +2792,7 @@ top.o: top.c $(defs_h) $(gdbcmd_h) $(cal
 	$(annotate_h) $(completer_h) $(top_h) $(version_h) $(serial_h) \
 	$(doublest_h) $(gdb_assert_h) $(readline_h) $(readline_history_h) \
 	$(event_top_h) $(gdb_string_h) $(gdb_stat_h) $(ui_out_h) \
-	$(cli_out_h) $(main_h)
+	$(cli_out_h) $(main_h) $(event_loop_h)
 tracepoint.o: tracepoint.c $(defs_h) $(symtab_h) $(frame_h) $(gdbtypes_h) \
 	$(expression_h) $(gdbcmd_h) $(value_h) $(target_h) $(language_h) \
 	$(gdb_string_h) $(inferior_h) $(tracepoint_h) $(remote_h) \
--- gdb/top.c	21 Jul 2006 14:46:53 -0000	1.115
+++ gdb/top.c	25 Dec 2006 22:46:43 -0000
@@ -47,6 +47,7 @@
 #include "doublest.h"
 #include "gdb_assert.h"
 #include "main.h"
+#include "event-loop.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -721,17 +722,79 @@ The filename in which to record the comm
    synchronous mode.  So for operate-and-get-next to work in this
    situation, we have to switch the hooks around.  That is what
    gdb_readline_wrapper is for.  */
+
+static char *gdb_readline_wrapper_result;
+
+static void gdb_readline_wrapper_line (char *line)
+{
+  gdb_assert (gdb_readline_wrapper_result == NULL);
+  gdb_readline_wrapper_result = line;
+}
+
+struct gdb_readline_wrapper_cleanup
+  {
+    void (*handler_orig) (char *);
+    char *prompt_orig;
+    int already_prompted_orig;
+  };
+
+static void gdb_readline_wrapper_cleanup (struct gdb_readline_wrapper_cleanup *cleanup)
+{
+  gdb_assert (rl_already_prompted == 1);
+  rl_already_prompted = cleanup->already_prompted_orig;
+  set_prompt (cleanup->prompt_orig);
+  /* TODO: `cleanup->prompt_orig' leaks.  */
+
+  gdb_assert (input_handler == gdb_readline_wrapper_line);
+  input_handler = cleanup->handler_orig;
+  gdb_readline_wrapper_result = NULL;
+
+  xfree (cleanup);
+}
+
 char *
 gdb_readline_wrapper (char *prompt)
 {
+  struct cleanup *old_cleanups;
+  struct gdb_readline_wrapper_cleanup *cleanup;
+  char *retval;
+
+  /* `rl_initialize' not called as we are already called from the main loop.  */
+
+  cleanup = xmalloc (sizeof (*cleanup));
+
   /* Set the hook that works in this case.  */
   if (after_char_processing_hook)
     {
       rl_pre_input_hook = (Function *) after_char_processing_hook;
       after_char_processing_hook = NULL;
     }
+  cleanup->handler_orig = input_handler;
+  input_handler = gdb_readline_wrapper_line;
+
+  cleanup->prompt_orig = get_prompt ();
+  set_prompt (prompt);
+  cleanup->already_prompted_orig = rl_already_prompted;
+
+  old_cleanups = make_cleanup ((make_cleanup_ftype *)
+			       gdb_readline_wrapper_cleanup, cleanup);
+
+  /* Install our `input_handler' and prevent double prompt display.
+     The second prompt would get otherwise displayed before
+     `gdb_readline_wrapper_cleanup' being called.  */
+  display_gdb_prompt (NULL);
+  rl_already_prompted = 1;
+
+  /* gdb_do_one_event () argument is unused.  */
+  while (gdb_do_one_event (NULL) >= 0)
+    if (gdb_readline_wrapper_result != NULL)
+      break;
+
+  retval = gdb_readline_wrapper_result;
+  gdb_assert (retval != NULL);
+  do_cleanups (old_cleanups);
 
-  return readline (prompt);
+  return retval;
 }
 
 \f
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.base/readline-callback-history.c	25 Dec 2006 22:54:37 -0000
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2006 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 2 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, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+
+   Please email any bugs, comments, and/or additions to this file to:
+   bug-gdb@prep.ai.mit.edu  */
+
+int main()
+{
+  return 0;
+}
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.base/readline-callback-history.exp	25 Dec 2006 22:54:37 -0000
@@ -0,0 +1,55 @@
+# Copyright 2006 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile start
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+
+# For: \033[A (up arrow)
+set env(TERM) vt100
+
+# Get things started.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# For C programs, "start" should stop in main().
+
+gdb_test "b main" \
+         "Breakpoint 1 at.*" \
+         "Breakpoint put"
+
+gdb_test "run" \
+         "Breakpoint 1, main (.*) at .*" \
+         "Stopped at the breakpoint"
+
+# \033[A (up arrow)
+gdb_test "command 1\n\033\[A\nend" \
+         "Type commands for when breakpoint 1 is hit.*\n>command 1.*" \
+         "History is available even from callback"

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Tue, Dec 26, 2006 at 07:00:28AM +0100, Jan Kratochvil wrote:
> Here is one which forks for me but still it is more complicated than I expected.
> 
> I believe there may be side effects but unfortunately it can be expected the
> interactive readline(3) usage is not well covered by the testsuite.
> readline(3) call does a lot of various initializations currently omitted by the
> callback-based gdb reimplementation.

I simplified your testcase a bit, and cleaned up code formatting.
Unfortunately, while testing it, I got failures for the other
readline tests in the testsuite (for operate-and-get-next).  The
result of operate-and-get-next gets written out before the prompt
instead of after.

The comments make it pretty clear why it doesn't work:

   At the ordinary top-level prompt we might be using the async
   readline.  That means we can't use rl_pre_input_hook, since it
   doesn't work properly in async mode.

So I also needed a bit of fiddling around with the hooks to get that
to pass.  Which gave me this patch.  I've tested it on
x86_64-pc-linux-gnu and checked it in.  GDB no longer contains any
calls to the synchronous readline().

-- 
Daniel Jacobowitz
CodeSourcery

2007-01-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (top.o): Update.
	* top.c (gdb_readline_wrapper_done, gdb_readline_wrapper_result)
	(saved_after_char_processing_hook, gdb_readline_wrapper_line)
	(struct gdb_readline_wrapper_cleanup, gdb_readline_wrapper_cleanup):
	New.
	(gdb_readline_wrapper): Rewrite to use asynchronous readline.

2007-01-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.base/readline.exp: Set $TERM.  Test arrow keys in
	secondary prompts.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.864
diff -u -p -r1.864 Makefile.in
--- Makefile.in	3 Jan 2007 18:05:43 -0000	1.864
+++ Makefile.in	3 Jan 2007 21:22:46 -0000
@@ -2782,7 +2782,7 @@ top.o: top.c $(defs_h) $(gdbcmd_h) $(cal
 	$(annotate_h) $(completer_h) $(top_h) $(version_h) $(serial_h) \
 	$(doublest_h) $(gdb_assert_h) $(readline_h) $(readline_history_h) \
 	$(event_top_h) $(gdb_string_h) $(gdb_stat_h) $(ui_out_h) \
-	$(cli_out_h) $(main_h)
+	$(cli_out_h) $(main_h) $(event_loop_h)
 tracepoint.o: tracepoint.c $(defs_h) $(symtab_h) $(frame_h) $(gdbtypes_h) \
 	$(expression_h) $(gdbcmd_h) $(value_h) $(target_h) $(language_h) \
 	$(gdb_string_h) $(inferior_h) $(tracepoint_h) $(remote_h) \
Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.116
diff -u -p -r1.116 top.c
--- top.c	1 Jan 2007 05:57:49 -0000	1.116
+++ top.c	3 Jan 2007 21:22:47 -0000
@@ -47,6 +47,7 @@
 #include "doublest.h"
 #include "gdb_assert.h"
 #include "main.h"
+#include "event-loop.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -712,26 +713,111 @@ The filename in which to record the comm
 }
 
 /* This is like readline(), but it has some gdb-specific behavior.
-   gdb can use readline in both the synchronous and async modes during
+   gdb may want readline in both the synchronous and async modes during
    a single gdb invocation.  At the ordinary top-level prompt we might
    be using the async readline.  That means we can't use
    rl_pre_input_hook, since it doesn't work properly in async mode.
    However, for a secondary prompt (" >", such as occurs during a
-   `define'), gdb just calls readline() directly, running it in
-   synchronous mode.  So for operate-and-get-next to work in this
-   situation, we have to switch the hooks around.  That is what
-   gdb_readline_wrapper is for.  */
+   `define'), gdb wants a synchronous response.
+
+   We used to call readline() directly, running it in synchronous
+   mode.  But mixing modes this way is not supported, and as of
+   readline 5.x it no longer works; the arrow keys come unbound during
+   the synchronous call.  So we make a nested call into the event
+   loop.  That's what gdb_readline_wrapper is for.  */
+
+/* A flag set as soon as gdb_readline_wrapper_line is called; we can't
+   rely on gdb_readline_wrapper_result, which might still be NULL if
+   the user types Control-D for EOF.  */
+static int gdb_readline_wrapper_done;
+
+/* The result of the current call to gdb_readline_wrapper, once a newline
+   is seen.  */
+static char *gdb_readline_wrapper_result;
+
+/* Any intercepted hook.  Operate-and-get-next sets this, expecting it
+   to be called after the newline is processed (which will redisplay
+   the prompt).  But in gdb_readline_wrapper we will not get a new
+   prompt until the next call, or until we return to the event loop.
+   So we disable this hook around the newline and restore it before we
+   return.  */
+static void (*saved_after_char_processing_hook) (void);
+
+/* This function is called when readline has seen a complete line of
+   text.  */
+
+static void
+gdb_readline_wrapper_line (char *line)
+{
+  gdb_assert (!gdb_readline_wrapper_done);
+  gdb_readline_wrapper_result = line;
+  gdb_readline_wrapper_done = 1;
+
+  /* Prevent operate-and-get-next from acting too early.  */
+  saved_after_char_processing_hook = after_char_processing_hook;
+  after_char_processing_hook = NULL;
+}
+
+struct gdb_readline_wrapper_cleanup
+  {
+    void (*handler_orig) (char *);
+    char *prompt_orig;
+    int already_prompted_orig;
+  };
+
+static void
+gdb_readline_wrapper_cleanup (void *arg)
+{
+  struct gdb_readline_wrapper_cleanup *cleanup = arg;
+
+  gdb_assert (rl_already_prompted == 1);
+  rl_already_prompted = cleanup->already_prompted_orig;
+  PROMPT (0) = cleanup->prompt_orig;
+
+  gdb_assert (input_handler == gdb_readline_wrapper_line);
+  input_handler = cleanup->handler_orig;
+  gdb_readline_wrapper_result = NULL;
+  gdb_readline_wrapper_done = 0;
+
+  after_char_processing_hook = saved_after_char_processing_hook;
+  saved_after_char_processing_hook = NULL;
+
+  xfree (cleanup);
+}
+
 char *
 gdb_readline_wrapper (char *prompt)
 {
-  /* Set the hook that works in this case.  */
+  struct cleanup *back_to;
+  struct gdb_readline_wrapper_cleanup *cleanup;
+  char *retval;
+
+  cleanup = xmalloc (sizeof (*cleanup));
+  cleanup->handler_orig = input_handler;
+  input_handler = gdb_readline_wrapper_line;
+
+  cleanup->prompt_orig = get_prompt ();
+  PROMPT (0) = prompt;
+  cleanup->already_prompted_orig = rl_already_prompted;
+
+  back_to = make_cleanup (gdb_readline_wrapper_cleanup, cleanup);
+
+  /* Display our prompt and prevent double prompt display.  */
+  display_gdb_prompt (NULL);
+  rl_already_prompted = 1;
+
   if (after_char_processing_hook)
-    {
-      rl_pre_input_hook = (Function *) after_char_processing_hook;
-      after_char_processing_hook = NULL;
-    }
+    (*after_char_processing_hook) ();
+  gdb_assert (after_char_processing_hook == NULL);
 
-  return readline (prompt);
+  /* gdb_do_one_event argument is unused.  */
+  while (gdb_do_one_event (NULL) >= 0)
+    if (gdb_readline_wrapper_done)
+      break;
+
+  retval = gdb_readline_wrapper_result;
+  do_cleanups (back_to);
+  return retval;
 }
 
 \f
Index: testsuite/gdb.base/readline.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/readline.exp,v
retrieving revision 1.2
diff -u -p -r1.2 readline.exp
--- testsuite/gdb.base/readline.exp	8 Jun 2003 13:14:05 -0000	1.2
+++ testsuite/gdb.base/readline.exp	3 Jan 2007 21:22:47 -0000
@@ -1,4 +1,4 @@
-# Copyright 2002 Free Software Foundation, Inc.
+# Copyright 2002, 2003, 2007 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
@@ -159,6 +159,14 @@ if [info exists env(INPUTRC)] {
 }
 set env(INPUTRC) "/dev/null"
 
+# The arrow key test relies on the standard VT100 bindings, so make
+# sure that an appropriate terminal is selected.  The same bug
+# doesn't show up if we use ^P / ^N instead.
+if [info exists env(TERM)] {
+    set old_term $env(TERM)
+}
+set env(TERM) "vt100"
+
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 
@@ -178,6 +186,18 @@ operate_and_get_next "operate-and-get-ne
   "p 5" "" \
   "end" ".* = 5"
 
+# Verify that arrow keys work in secondary prompts.  The control
+# sequence is a hard-coded VT100 up arrow.
+gdb_test "print 42" "\\\$\[0-9\]* = 42"
+set msg "arrow keys with secondary prompt"
+gdb_test_multiple "if 1 > 0\n\033\[A\033\[A\nend" $msg {
+    -re ".*\\\$\[0-9\]* = 42\r\n$gdb_prompt $" {
+	pass $msg
+    }
+    -re ".*Undefined command:.*$gdb_prompt $" {
+	fail $msg
+    }
+}
 
 # Now repeat the first test with a history file that fills the entire
 # history list.

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Wed, Jan 03, 2007 at 04:46:40PM -0500, Daniel Jacobowitz wrote:
> So I also needed a bit of fiddling around with the hooks to get that
> to pass.  Which gave me this patch.  I've tested it on
> x86_64-pc-linux-gnu and checked it in.  GDB no longer contains any
> calls to the synchronous readline().

Oh, and: thanks for putting this together.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Nick Roberts]

 > > This change breaks the behaviour of annotations with commands that span
 > > multiple lines, like if, while, etc:
 > 
 > Sorry, I didn't even know we had special annotations for this (and the
 > testsuite must not cover it)...

Here's a test for gdb.base/annota3.exp.  I can do the same for
gdb.base/annota1.exp and gdb.cp/annota3.exp, if needed.

-- 
Nick                                           http://www.inet.net.nz/~nickrob



2007-06-06  Nick Roberts  <nickrob@snap.net.nz>

	* gdb.base/annota3.exp: Test for if construct.

*** annota3.exp	10 Jan 2007 06:59:09 +1300	1.12
--- annota3.exp	06 Jun 2007 00:50:22 +1200	
*************** gdb_expect_list "annotation set at level
*** 99,105 ****
      "set annotate 3"
  }
  
! 
  #
  # info break:
  #
--- 99,121 ----
      "set annotate 3"
  }
  
! #
! # if construct:
! #
! send_gdb "if 1\n"
! gdb_expect {
!     -re "\r\n\032\032post-prompt\r\n\r\n\032\032pre-commands\r\n >\r\n\032\032commands\r\n" {
! 	pass "start if construct"
!     }
!     timeout { fail "start if construct (timeout)" }
! }
! send_gdb "end\n"
! gdb_expect {
!     -re  "\r\n\032\032post-commands\r\n$gdb_prompt$" {
!  	pass "end if construct"
!     }
!     timeout { fail "end if construct (timeout)" }
! }
  #
  # info break:
  #

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Wed, Jun 06, 2007 at 12:55:55AM +1200, Nick Roberts wrote:
>  > > This change breaks the behaviour of annotations with commands that span
>  > > multiple lines, like if, while, etc:
>  > 
>  > Sorry, I didn't even know we had special annotations for this (and the
>  > testsuite must not cover it)...
> 
> Here's a test for gdb.base/annota3.exp.  I can do the same for
> gdb.base/annota1.exp and gdb.cp/annota3.exp, if needed.

Thanks.  This is OK once we fix the problem.  I haven't gotten back to
you about that yet because I want to figure out what other issues were
solved by the original patch - there were several.  But I've been on
vacation and I'm in a mad rush at home now, so it may be a bit longer.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Tue, Jun 05, 2007 at 09:27:00AM -0400, Daniel Jacobowitz wrote:
> On Wed, Jun 06, 2007 at 12:55:55AM +1200, Nick Roberts wrote:
> >  > > This change breaks the behaviour of annotations with commands that span
> >  > > multiple lines, like if, while, etc:
> >  > 
> >  > Sorry, I didn't even know we had special annotations for this (and the
> >  > testsuite must not cover it)...
> > 
> > Here's a test for gdb.base/annota3.exp.  I can do the same for
> > gdb.base/annota1.exp and gdb.cp/annota3.exp, if needed.
> 
> Thanks.  This is OK once we fix the problem.  I haven't gotten back to
> you about that yet because I want to figure out what other issues were
> solved by the original patch - there were several.  But I've been on
> vacation and I'm in a mad rush at home now, so it may be a bit longer.

Nick, does this work for emacs?  It fixes the testcase (which I
updated to fail and not time out for broken GDBs).

-- 
Daniel Jacobowitz
CodeSourcery

2007-06-24  Daniel Jacobowitz  <dan@codesourcery.com>

	* top.c (gdb_readline_wrapper_line): Call rl_callback_handler_remove.
	(struct gdb_readline_wrapper_cleanup): Remove prompt_orig.
	(gdb_readline_wrapper_cleanup): Do not reset the prompt.
	(gdb_readline_wrapper): Do not save the prompt.  Pass our prompt
	to display_gdb_prompt.

2007-06-24  Nick Roberts  <nickrob@snap.net.nz>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.base/annota3.exp: Test for if construct.

Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.120
diff -u -p -r1.120 top.c
--- top.c	29 Mar 2007 18:55:01 -0000	1.120
+++ top.c	24 Jun 2007 15:42:02 -0000
@@ -751,12 +751,16 @@ gdb_readline_wrapper_line (char *line)
   /* Prevent operate-and-get-next from acting too early.  */
   saved_after_char_processing_hook = after_char_processing_hook;
   after_char_processing_hook = NULL;
+
+  /* Prevent parts of the prompt from being redisplayed if annotations
+     are enabled, and readline's state getting out of sync.  */
+  if (async_command_editing_p)
+    rl_callback_handler_remove ();
 }
 
 struct gdb_readline_wrapper_cleanup
   {
     void (*handler_orig) (char *);
-    char *prompt_orig;
     int already_prompted_orig;
   };
 
@@ -766,7 +770,6 @@ gdb_readline_wrapper_cleanup (void *arg)
   struct gdb_readline_wrapper_cleanup *cleanup = arg;
 
   rl_already_prompted = cleanup->already_prompted_orig;
-  PROMPT (0) = cleanup->prompt_orig;
 
   gdb_assert (input_handler == gdb_readline_wrapper_line);
   input_handler = cleanup->handler_orig;
@@ -790,14 +793,12 @@ gdb_readline_wrapper (char *prompt)
   cleanup->handler_orig = input_handler;
   input_handler = gdb_readline_wrapper_line;
 
-  cleanup->prompt_orig = get_prompt ();
-  PROMPT (0) = prompt;
   cleanup->already_prompted_orig = rl_already_prompted;
 
   back_to = make_cleanup (gdb_readline_wrapper_cleanup, cleanup);
 
   /* Display our prompt and prevent double prompt display.  */
-  display_gdb_prompt (NULL);
+  display_gdb_prompt (prompt);
   rl_already_prompted = 1;
 
   if (after_char_processing_hook)
Index: testsuite/gdb.base/annota3.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/annota3.exp,v
retrieving revision 1.12
diff -u -p -r1.12 annota3.exp
--- testsuite/gdb.base/annota3.exp	9 Jan 2007 17:59:09 -0000	1.12
+++ testsuite/gdb.base/annota3.exp	24 Jun 2007 15:42:02 -0000
@@ -99,7 +99,29 @@ gdb_expect_list "annotation set at level
     "set annotate 3"
 }
 
-
+#
+# if construct:
+#
+send_gdb "if 1\n"
+gdb_expect {
+    -re "^if 1\r\n\r\n\032\032post-prompt\r\n\r\n\032\032pre-commands\r\n >\r\n\032\032commands\r\n$" {
+	pass "start if construct"
+    }
+    -re ".*\032\032commands\r\n" {
+	fail "start if construct"
+    }
+    timeout { fail "start if construct (timeout)" }
+}
+send_gdb "end\n"
+gdb_expect {
+    -re "^end\r\n\r\n\032\032post-commands\r\n$gdb_prompt$" {
+ 	pass "end if construct"
+    }
+    -re ".*$gdb_prompt$" {
+	fail "end if construct"
+    }
+    timeout { fail "end if construct (timeout)" }
+}
 #
 # info break:
 #

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Nick Roberts]

 > Nick, does this work for emacs?  It fixes the testcase (which I
 > updated to fail and not time out for broken GDBs).

Yes, this looks good to me.

Thanks,

Nick

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Mon, Jun 25, 2007 at 09:42:24AM +1200, Nick Roberts wrote:
>  > Nick, does this work for emacs?  It fixes the testcase (which I
>  > updated to fail and not time out for broken GDBs).
> 
> Yes, this looks good to me.
> 
> Thanks,

Thank you for your patience.  I've checked it in; GDB 6.7 will not
have the bug.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Nick Roberts]

 > >  > Nick, does this work for emacs?  It fixes the testcase (which I
 > >  > updated to fail and not time out for broken GDBs).
 > > 
 > > Yes, this looks good to me.
 > > 
 > > Thanks,
 > 
 > Thank you for your patience.  I've checked it in; GDB 6.7 will not
 > have the bug.

Thanks.  Are you going to update the Wiki?  More generally, is the Wiki page
for the GDB 6.7 release otherwise up-to-date?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Tue, Jul 03, 2007 at 01:28:05PM +1200, Nick Roberts wrote:
>  > >  > Nick, does this work for emacs?  It fixes the testcase (which I
>  > >  > updated to fail and not time out for broken GDBs).
>  > > 
>  > > Yes, this looks good to me.
>  > > 
>  > > Thanks,
>  > 
>  > Thank you for your patience.  I've checked it in; GDB 6.7 will not
>  > have the bug.
> 
> Thanks.  Are you going to update the Wiki?  More generally, is the Wiki page
> for the GDB 6.7 release otherwise up-to-date?

It's a little stale; I'm planning to update it in the morning, after I
finish going through the patches I'd flagged.  I'd do it now but I
don't have my password on this computer.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Tue, Jul 03, 2007 at 01:28:05PM +1200, Nick Roberts wrote:
> Thanks.  Are you going to update the Wiki?  More generally, is the Wiki page
> for the GDB 6.7 release otherwise up-to-date?

It's up to date now.  I'll do the two remaining issues with patches,
then look into signed char again.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]

Hi,

Chet, the patch would be ported later from the current GDB readline-5.1 to 5.2.


On Mon, 04 Jun 2007 03:06:33 +0200, Daniel Jacobowitz wrote:
> On Mon, Jun 04, 2007 at 10:54:25AM +1200, Nick Roberts wrote:
> > 
> > This change breaks the behaviour of annotations with commands that span
> > multiple lines, like if, while, etc:
> 
> Sorry, I didn't even know we had special annotations for this (and the
> testsuite must not cover it).  I'm not sure how to fix it, either -
> the patch you cited was very tricky to come up with already :-(

The regression happened as the patch was very complex:
	http://sourceware.org/ml/gdb-cvs/2007-01/msg00022.html

The synchronous readline() call has no asynchronous counterpart in libreadline.
Any asynchronous readline() emulation on top of the current libreadline API may
work but it is hard to make it precisely backward compatible.

Facts review:

* longjmp() out of readline() is so far documented API usage of libreadline:
  	@node Readline Signal Handling
  	If the application's signal handler does more than update its idea of
  	the terminal size and return (for example, a @code{longjmp} back to
  	a main processing loop),

* Application (GDB) cannot maintain the RL_STATE_CALLBACK flag as it is
  undocumented and therefore internal to libreadline.

* Any (non-unwinder) based libreadline code cannot set the RL_STATE_CALLBACK
  flag properly as it cannot find if the application longjmp()ed into a stack
  depth originally being in callback or non-callback mode.  Proof pseudocode:
  	http://sources.redhat.com/ml/gdb-patches/2006-12/msg00224.html

Multiple solutions exist:

(1) Code a new libreadline API function readline_async() which will try to
    be backward compatible with readline().
    - Such function complies with active RL_STATE_CALLBACK requreiment but
      a synchronous function would better fit the current use in GDB.

(2) Provide a new documented way to abort readline() from a signal handler.
    Provided rl_readline_unwind() needed to be called right before longjmp().
    Proposed, patch attached.
    + Fully crossplatform, backward compatible.
    + Not affecting most of the apps as they do not longjmp() out of readline().
    - New libreadline API function.

(3) Provide C++ try/catch handling (attached, GDB -lstd++ needed, deprecated).
    + Transparent to the application.
    - libreadline.a has no backward compatible linking (-lstdc++ is needed)
    - Systems without C++ will link but the current bug remains present there.

(4) Custom C based unwinder registration (such as C++ but without -lstdc++)
    - gcc dependent, generally noncrossplatform.


Regards,
Jan

[-- Attachment #2: Proposed patch implementing rl_readline_unwind(). --]
[-- Type: text/plain, Size: 5149 bytes --]

2007-06-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* top.c (gdb_readline_wrapper) Wrap readline() by readline_cleanup().
	(readline_cleanup): New function.

2007-06-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    H.J. Lu  <hongjiu.lu@intel.com>

	PR tui/2173
	* readline.c (readline): Save/restore RL_STATE_CALLBACK into STATE_LIST.
	(rl_readline_unwind, struct state, state_list): New.
	* readline.h (rl_readline_unwind): New declaration.

2007-06-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* rltech.texi (Readline Signal Handling): Describe rl_readline_unwind().

--- sources-unpatched/gdb/top.c	2007-06-26 13:04:11.000000000 +0200
+++ sources/gdb/top.c	2007-06-26 12:38:43.000000000 +0200
@@ -706,6 +706,13 @@ The filename in which to record the comm
 		    value);
 }
 
+static void
+readline_cleanup (void *unused)
+{
+  /* No need to check for it by autoconf as libreadline is bundled with GDB.  */
+  rl_readline_unwind ();
+}
+
 /* This is like readline(), but it has some gdb-specific behavior.
    gdb can use readline in both the synchronous and async modes during
    a single gdb invocation.  At the ordinary top-level prompt we might
@@ -719,6 +726,9 @@ The filename in which to record the comm
 char *
 gdb_readline_wrapper (char *prompt)
 {
+  char *retval;
+  struct cleanup *cleanups;
+
   /* Set the hook that works in this case.  */
   if (after_char_processing_hook)
     {
@@ -726,7 +736,11 @@ gdb_readline_wrapper (char *prompt)
       after_char_processing_hook = NULL;
     }
 
-  return readline (prompt);
+  cleanups = make_cleanup (readline_cleanup, NULL);
+  retval = readline (prompt);
+  discard_cleanups (cleanups);
+
+  return retval;
 }
 
 \f
--- sources-unpatched/readline/doc/rltech.texi	2007-06-26 12:54:57.000000000 +0200
+++ sources/readline/doc/rltech.texi	2007-06-26 12:40:50.000000000 +0200
@@ -1336,7 +1336,16 @@ resetting the terminal to its original s
 handler does more than update its idea of the terminal size and return (for
 example, a @code{longjmp} back to a main processing loop), it @emph{must}
 call @code{rl_cleanup_after_signal()} (described below), to restore the
-terminal state. 
+terminal state.  Each @code{longjmp} call must be also protected by
+@code{rl_readline_unwind()}:
+
+@deftypefun void rl_readline_unwind (void)
+Each @code{readline()} call you jump out of by @code{longjmp} must be matched
+by one call of this function.  It is appropriate to call it from your
+@code{catch} part of the C++ exception handler.  This functiona must not be
+called if @code{readline()} returns itself.  If you do not abort
+@code{readline()} by @code{longjmp} you must never call this function.
+@end deftypefun
 
 Readline provides two variables that allow application writers to
 control whether or not it will catch certain signals and act on them
--- sources-unpatched/readline/readline.c	2007-06-26 12:54:52.000000000 +0200
+++ sources/readline/readline.c	2007-06-26 12:33:57.000000000 +0200
@@ -287,7 +287,27 @@ rl_set_prompt (prompt)
   rl_visible_prompt_length = rl_expand_prompt (rl_prompt);
   return 0;
 }
-  
+
+struct state {
+  struct state *next;
+  int in_callback;
+};
+static struct state *state_list;
+
+/* To be called once for each readline () call which you have jumped out of by
+   longjmp() or siglongjmp().  Function needs to be called BEFORE the jump.  */
+void
+rl_readline_unwind ()
+{
+  /* NULL should not happen.  */
+  if (state_list != NULL)
+    {
+      if (state_list->in_callback)
+	RL_SETSTATE (RL_STATE_CALLBACK);
+      state_list = state_list->next;
+    }
+}
+
 /* Read a line of input.  Prompt with PROMPT.  An empty PROMPT means
    none.  A return value of NULL means that EOF was encountered. */
 char *
@@ -295,6 +315,7 @@ readline (prompt)
      const char *prompt;
 {
   char *value;
+  struct state state;
 
   /* If we are at EOF return a NULL string. */
   if (rl_pending_input == EOF)
@@ -303,6 +324,15 @@ readline (prompt)
       return ((char *)NULL);
     }
 
+  /* When we call readline, we have to make sure that readline isn't in
+     the callback state. Otherwise, it will get really confused.
+     PR gdb tui/2173.  */
+  state.next = state_list;
+  state_list = &state;
+  state.in_callback = RL_ISSTATE (RL_STATE_CALLBACK);
+  if (state.in_callback)
+    RL_UNSETSTATE (RL_STATE_CALLBACK);
+
   rl_set_prompt (prompt);
 
   rl_initialize ();
@@ -321,6 +351,8 @@ readline (prompt)
   rl_clear_signals ();
 #endif
 
+  rl_readline_unwind ();
+
   return (value);
 }
 
--- sources-unpatched/readline/readline.h	2007-06-26 12:54:52.000000000 +0200
+++ sources/readline/readline.h	2007-06-26 12:34:00.000000000 +0200
@@ -280,6 +280,10 @@ extern int rl_vi_eword PARAMS((int, int)
 /* Read a line of input.  Prompt with PROMPT.  A NULL PROMPT means none. */
 extern char *readline PARAMS((const char *));
 
+/* To be called once for each readline () call which you have jumped out of by
+   longjmp() or siglongjmp().  Function needs to be called BEFORE the jump.  */
+extern void rl_readline_unwind PARAMS((void));
+
 extern int rl_set_prompt PARAMS((const char *));
 extern int rl_expand_prompt PARAMS((char *));
 

[-- Attachment #3: Revert of the so far committed patch. --]
[-- Type: text/plain, Size: 5936 bytes --]

2007-06-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* top.c (gdb_readline_wrapper_done, gdb_readline_wrapper_result)
	(saved_after_char_processing_hook, gdb_readline_wrapper_line)
	(struct gdb_readline_wrapper_cleanup, gdb_readline_wrapper_cleanup)
	(gdb_readline_wrapper): Revert the asynchronous readline patches from
	2007-02-28 and 2007-01-03 back to the synchronous state.
	* Makefile.in (top.o): Likewise.

--- ./gdb/top.c	28 Feb 2007 15:55:54 -0000	1.119
+++ ./gdb/top.c	9 Jan 2007 17:58:59 -0000	1.118
@@ -770,6 +770,7 @@ gdb_readline_wrapper_cleanup (void *arg)
 {
   struct gdb_readline_wrapper_cleanup *cleanup = arg;
 
+  gdb_assert (rl_already_prompted == 1);
   rl_already_prompted = cleanup->already_prompted_orig;
   PROMPT (0) = cleanup->prompt_orig;
 
--- ./gdb/Makefile.in	3 Jan 2007 21:46:11 -0000	1.865
+++ ./gdb/Makefile.in	3 Jan 2007 18:05:43 -0000	1.864
@@ -2782,7 +2782,7 @@ top.o: top.c $(defs_h) $(gdbcmd_h) $(cal
 	$(annotate_h) $(completer_h) $(top_h) $(version_h) $(serial_h) \
 	$(doublest_h) $(gdb_assert_h) $(readline_h) $(readline_history_h) \
 	$(event_top_h) $(gdb_string_h) $(gdb_stat_h) $(ui_out_h) \
-	$(cli_out_h) $(main_h) $(event_loop_h)
+	$(cli_out_h) $(main_h)
 tracepoint.o: tracepoint.c $(defs_h) $(symtab_h) $(frame_h) $(gdbtypes_h) \
 	$(expression_h) $(gdbcmd_h) $(value_h) $(target_h) $(language_h) \
 	$(gdb_string_h) $(inferior_h) $(tracepoint_h) $(remote_h) \
--- ./gdb/top.c	3 Jan 2007 21:46:11 -0000	1.117
+++ ./gdb/top.c	1 Jan 2007 05:57:49 -0000	1.116
@@ -47,7 +47,6 @@
 #include "doublest.h"
 #include "gdb_assert.h"
 #include "main.h"
-#include "event-loop.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -713,111 +712,26 @@ The filename in which to record the comm
 }
 
 /* This is like readline(), but it has some gdb-specific behavior.
-   gdb may want readline in both the synchronous and async modes during
+   gdb can use readline in both the synchronous and async modes during
    a single gdb invocation.  At the ordinary top-level prompt we might
    be using the async readline.  That means we can't use
    rl_pre_input_hook, since it doesn't work properly in async mode.
    However, for a secondary prompt (" >", such as occurs during a
-   `define'), gdb wants a synchronous response.
-
-   We used to call readline() directly, running it in synchronous
-   mode.  But mixing modes this way is not supported, and as of
-   readline 5.x it no longer works; the arrow keys come unbound during
-   the synchronous call.  So we make a nested call into the event
-   loop.  That's what gdb_readline_wrapper is for.  */
-
-/* A flag set as soon as gdb_readline_wrapper_line is called; we can't
-   rely on gdb_readline_wrapper_result, which might still be NULL if
-   the user types Control-D for EOF.  */
-static int gdb_readline_wrapper_done;
-
-/* The result of the current call to gdb_readline_wrapper, once a newline
-   is seen.  */
-static char *gdb_readline_wrapper_result;
-
-/* Any intercepted hook.  Operate-and-get-next sets this, expecting it
-   to be called after the newline is processed (which will redisplay
-   the prompt).  But in gdb_readline_wrapper we will not get a new
-   prompt until the next call, or until we return to the event loop.
-   So we disable this hook around the newline and restore it before we
-   return.  */
-static void (*saved_after_char_processing_hook) (void);
-
-/* This function is called when readline has seen a complete line of
-   text.  */
-
-static void
-gdb_readline_wrapper_line (char *line)
-{
-  gdb_assert (!gdb_readline_wrapper_done);
-  gdb_readline_wrapper_result = line;
-  gdb_readline_wrapper_done = 1;
-
-  /* Prevent operate-and-get-next from acting too early.  */
-  saved_after_char_processing_hook = after_char_processing_hook;
-  after_char_processing_hook = NULL;
-}
-
-struct gdb_readline_wrapper_cleanup
-  {
-    void (*handler_orig) (char *);
-    char *prompt_orig;
-    int already_prompted_orig;
-  };
-
-static void
-gdb_readline_wrapper_cleanup (void *arg)
-{
-  struct gdb_readline_wrapper_cleanup *cleanup = arg;
-
-  gdb_assert (rl_already_prompted == 1);
-  rl_already_prompted = cleanup->already_prompted_orig;
-  PROMPT (0) = cleanup->prompt_orig;
-
-  gdb_assert (input_handler == gdb_readline_wrapper_line);
-  input_handler = cleanup->handler_orig;
-  gdb_readline_wrapper_result = NULL;
-  gdb_readline_wrapper_done = 0;
-
-  after_char_processing_hook = saved_after_char_processing_hook;
-  saved_after_char_processing_hook = NULL;
-
-  xfree (cleanup);
-}
-
+   `define'), gdb just calls readline() directly, running it in
+   synchronous mode.  So for operate-and-get-next to work in this
+   situation, we have to switch the hooks around.  That is what
+   gdb_readline_wrapper is for.  */
 char *
 gdb_readline_wrapper (char *prompt)
 {
-  struct cleanup *back_to;
-  struct gdb_readline_wrapper_cleanup *cleanup;
-  char *retval;
-
-  cleanup = xmalloc (sizeof (*cleanup));
-  cleanup->handler_orig = input_handler;
-  input_handler = gdb_readline_wrapper_line;
-
-  cleanup->prompt_orig = get_prompt ();
-  PROMPT (0) = prompt;
-  cleanup->already_prompted_orig = rl_already_prompted;
-
-  back_to = make_cleanup (gdb_readline_wrapper_cleanup, cleanup);
-
-  /* Display our prompt and prevent double prompt display.  */
-  display_gdb_prompt (NULL);
-  rl_already_prompted = 1;
-
+  /* Set the hook that works in this case.  */
   if (after_char_processing_hook)
-    (*after_char_processing_hook) ();
-  gdb_assert (after_char_processing_hook == NULL);
+    {
+      rl_pre_input_hook = (Function *) after_char_processing_hook;
+      after_char_processing_hook = NULL;
+    }
 
-  /* gdb_do_one_event argument is unused.  */
-  while (gdb_do_one_event (NULL) >= 0)
-    if (gdb_readline_wrapper_done)
-      break;
-
-  retval = gdb_readline_wrapper_result;
-  do_cleanups (back_to);
-  return retval;
+  return readline (prompt);
 }
 
 \f

[-- Attachment #4: Preview of the deprecated C++ libreadline patch. --]
[-- Type: text/plain, Size: 7322 bytes --]

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/readline/Makefile.in,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile.in
--- Makefile.in	27 Mar 2007 18:09:35 -0000	1.9
+++ Makefile.in	26 Jun 2007 09:42:55 -0000
@@ -36,6 +36,7 @@ INSTALL_PROGRAM = @INSTALL_PROGRAM@
 INSTALL_DATA = @INSTALL_DATA@
 
 CC = @CC@
+CXX = @CXX@
 RANLIB = @RANLIB@
 AR = @AR@
 ARFLAGS = @ARFLAGS@
@@ -70,6 +71,7 @@ ETAGS = etags -tw
 CTAGS = ctags -tw
 
 CFLAGS = @CFLAGS@
+CXXFLAGS = @CXXFLAGS@
 LOCAL_CFLAGS = @LOCAL_CFLAGS@ -DRL_LIBRARY_VERSION='"$(RL_LIBRARY_VERSION)"'
 CPPFLAGS = @CPPFLAGS@
 
@@ -94,12 +96,21 @@ GCC_LINT_CFLAGS = $(XCCFLAGS) $(GCC_LINT
 	${RM} $@
 	$(CC) -c $(CCFLAGS) $<
 
+.C.o:
+	${RM} $@
+	$(CXX) -c $(CXXFLAGS) $<
+
 # The name of the main library target.
 LIBRARY_NAME = libreadline.a
 STATIC_LIBS = libreadline.a libhistory.a
 
 WCWIDTH_OBJ = @WCWIDTH_OBJ@
 
+@CPLUSPLUS_TRUE@OPTCSOURCES = cleanup-unwind.C
+@CPLUSPLUS_TRUE@OPTOBJECTS = cleanup-unwind.o
+@CPLUSPLUS_FALSE@OPTCSOURCES = cleanup-none.c
+@CPLUSPLUS_FALSE@OPTOBJECTS = cleanup-none.o
+
 # The C code source files for this library.
 CSOURCES = $(srcdir)/readline.c $(srcdir)/funmap.c $(srcdir)/keymaps.c \
 	   $(srcdir)/vi_mode.c $(srcdir)/parens.c $(srcdir)/rltty.c \
@@ -112,13 +123,13 @@ CSOURCES = $(srcdir)/readline.c $(srcdir
 	   $(srcdir)/histfile.c $(srcdir)/nls.c $(srcdir)/search.c \
 	   $(srcdir)/shell.c $(srcdir)/savestring.c $(srcdir)/tilde.c \
 	   $(srcdir)/text.c $(srcdir)/misc.c $(srcdir)/compat.c \
-	   $(srcdir)/mbutil.c $(srcdir)/support/wcwidth.c
+	   $(srcdir)/mbutil.c $(srcdir)/support/wcwidth.c $(OPTCSOURCES)
 
 # The header files for this library.
 HSOURCES = readline.h rldefs.h chardefs.h keymaps.h history.h histlib.h \
 	   posixstat.h posixdir.h posixjmp.h tilde.h rlconf.h rltty.h \
 	   ansi_stdlib.h tcap.h rlstdc.h xmalloc.h rlprivate.h rlshell.h \
-	   rltypedefs.h rlmbutil.h
+	   rltypedefs.h rlmbutil.h cleanup.h
 
 HISTOBJ = history.o histexpand.o histfile.o histsearch.o shell.o mbutil.o
 TILDEOBJ = tilde.o
@@ -126,7 +137,7 @@ OBJECTS = readline.o vi_mode.o funmap.o 
 	  rltty.o complete.o bind.o isearch.o display.o signals.o \
 	  util.o kill.o undo.o macro.o input.o callback.o terminal.o \
 	  text.o nls.o misc.o compat.o xmalloc.o $(HISTOBJ) $(TILDEOBJ) \
-	  $(WCWIDTH_OBJ)
+	  $(WCWIDTH_OBJ) $(OPTOBJECTS)
 
 # The texinfo files which document this library.
 DOCSOURCE = doc/rlman.texinfo doc/rltech.texinfo doc/rluser.texinfo
@@ -484,6 +495,10 @@ search.o: rlmbutil.h 
 text.o: rlmbutil.h
 vi_mode.o: rlmbutil.h
 
+readline.o: cleanup.h
+cleanup-none.o: cleanup.h
+cleanup-unwind.o: cleanup.h
+
 bind.o: $(srcdir)/bind.c
 callback.o: $(srcdir)/callback.c
 compat.o: $(srcdir)/compat.c
@@ -518,6 +533,9 @@ histfile.o: $(srcdir)/histfile.c
 history.o: $(srcdir)/history.c
 histsearch.o: $(srcdir)/histsearch.c
 
+cleanup-none.o: $(srcdir)/cleanup-none.c
+cleanup-unwind.o: $(srcdir)/cleanup-unwind.C
+
 bind.o: bind.c
 callback.o: callback.c
 compat.o: compat.c
@@ -551,3 +569,6 @@ histexpand.o: histexpand.c
 histfile.o: histfile.c
 history.o: history.c
 histsearch.o: histsearch.c
+
+cleanup-none.o: cleanup-none.c
+cleanup-unwind.o: cleanup-unwind.C
Index: cleanup-none.c
===================================================================
RCS file: cleanup-none.c
diff -N cleanup-none.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ cleanup-none.c	26 Jun 2007 09:42:55 -0000
@@ -0,0 +1,11 @@
+#include "cleanup.h"
+
+void *_rl_cleanup (void *(*body) (void *data), void (*cleanup) (void *data),
+		   void *data)
+{
+  void *retval;
+  
+  retval = (*body) (data);
+  (*cleanup) (data);
+  return retval;
+}
Index: cleanup-unwind.C
===================================================================
RCS file: cleanup-unwind.C
diff -N cleanup-unwind.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ cleanup-unwind.C	26 Jun 2007 09:42:55 -0000
@@ -0,0 +1,14 @@
+#include "cleanup.h"
+
+void *_rl_cleanup (void *(*body) (void *data), void (*cleanup) (void *data),
+		   void *data)
+{
+  try
+    {
+      return (*body) (data);
+    }
+  catch (...)
+    {
+      (*cleanup) (data);
+    }
+}
Index: cleanup.h
===================================================================
RCS file: cleanup.h
diff -N cleanup.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ cleanup.h	26 Jun 2007 09:42:55 -0000
@@ -0,0 +1,8 @@
+#ifdef  __cplusplus
+extern "C" {
+#endif
+void *_rl_cleanup (void *(*body) (void *data), void (*cleanup) (void *data),
+		   void *data);
+#ifdef  __cplusplus
+}
+#endif
Index: configure.in
===================================================================
RCS file: /cvs/src/src/readline/configure.in,v
retrieving revision 1.10
diff -u -p -r1.10 configure.in
--- configure.in	5 May 2006 18:26:12 -0000	1.10
+++ configure.in	26 Jun 2007 09:42:56 -0000
@@ -112,6 +112,26 @@ AC_PROG_CC
 dnl AC_AIX
 AC_MINIX
 
+m4_pushdef([AC_MSG_ERROR], [cxx_error=yes])
+AC_PROG_CXX
+AC_LANG_PUSH([C++])
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[const char hw[] = "hello world\n";]],
+				   [[cout << hw;]])],
+		  [AC_PROG_CXXCPP
+		   cxx_available=no],
+		  [cxx_available=yes])
+AC_LANG_POP([C++])
+m4_popdef([AC_MSG_ERROR])
+if test "x$cxx_available" = "xyes"; then
+    CPLUSPLUS_TRUE=""
+    CPLUSPLUS_FALSE="#"
+else
+    CPLUSPLUS_TRUE="#"
+    CPLUSPLUS_FALSE=""
+fi
+AC_SUBST(CPLUSPLUS_TRUE)
+AC_SUBST(CPLUSPLUS_FALSE)
+
 dnl BEGIN changes for CYGNUS cross-building for Cygwin
 dnl NOTE: Some of these changes may no longer be necessary.
 
Index: readline.c
===================================================================
RCS file: /cvs/src/src/readline/readline.c,v
retrieving revision 1.10
diff -u -p -r1.10 readline.c
--- readline.c	5 May 2006 18:26:12 -0000	1.10
+++ readline.c	26 Jun 2007 09:42:56 -0000
@@ -66,6 +66,7 @@
 #include "rlprivate.h"
 #include "rlshell.h"
 #include "xmalloc.h"
+#include "cleanup.h"
 
 #ifndef RL_LIBRARY_VERSION
 #  define RL_LIBRARY_VERSION "5.1"
@@ -287,6 +288,14 @@ rl_set_prompt (prompt)
   rl_visible_prompt_length = rl_expand_prompt (rl_prompt);
   return 0;
 }
+
+static void
+readline_cleanup (in_callback_pointer)
+     int *in_callback_pointer;
+{
+  if (*in_callback_pointer)
+    RL_SETSTATE (RL_STATE_CALLBACK);
+}
   
 /* Read a line of input.  Prompt with PROMPT.  An empty PROMPT means
    none.  A return value of NULL means that EOF was encountered. */
@@ -295,6 +304,7 @@ readline (prompt)
      const char *prompt;
 {
   char *value;
+  int in_callback;
 
   /* If we are at EOF return a NULL string. */
   if (rl_pending_input == EOF)
@@ -303,6 +313,13 @@ readline (prompt)
       return ((char *)NULL);
     }
 
+  /* When we call readline, we have to make sure that readline isn't in
+     the callback state. Otherwise, it will get really confused.
+     PR gdb tui/2173.  */
+  in_callback = RL_ISSTATE (RL_STATE_CALLBACK);
+  if (in_callback)
+    RL_UNSETSTATE (RL_STATE_CALLBACK);
+
   rl_set_prompt (prompt);
 
   rl_initialize ();
@@ -313,7 +330,10 @@ readline (prompt)
   rl_set_signals ();
 #endif
 
-  value = readline_internal ();
+  /* readline_internal () has no parameters.  */
+  value = _rl_cleanup ((void *(*) ()) readline_internal, readline_cleanup,
+		       &in_callback);
+
   if (rl_deprep_term_function)
     (*rl_deprep_term_function) ();
 

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Tue, Jun 26, 2007 at 03:13:37PM +0200, Jan Kratochvil wrote:
> The regression happened as the patch was very complex:
> 	http://sourceware.org/ml/gdb-cvs/2007-01/msg00022.html
> 
> The synchronous readline() call has no asynchronous counterpart in libreadline.
> Any asynchronous readline() emulation on top of the current libreadline API may
> work but it is hard to make it precisely backward compatible.

I've already posted a fix for this that doesn't involve changing
readline, FYI.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Jan Kratochvil]

Reply to:
http://sourceware.org/ml/gdb-patches/2007-06/msg00458.html

On Tue, 26 Jun 2007 15:13:37 +0200, Jan Kratochvil wrote:
...
> (2) Provide a new documented way to abort readline() from a signal handler.
>     Provided rl_readline_unwind() needed to be called right before longjmp().
>     Proposed, patch attached.
>     + Fully crossplatform, backward compatible.
>     + Not affecting most of the apps as they do not longjmp() out of readline().
>     - New libreadline API function.

Upon a recent discussion with the readline maintainer Chet Ramey I was told the
support for this method in fact exists in readline:

On Fri, 21 Mar 2008 20:01:51 +0100, Chet Ramey wrote:
> I'm looking at the rl_unwind_protect thing a little more closely, and I'm
> wondering why you didn't use rl_save_state and rl_restore_state
[snip]

These functions are just not in the info document.
They could be used to implement the GDB part of the patch
	http://sourceware.org/ml/gdb-patches/2007-06/txt00005.txt
but it needs to undo the current async-readline patch from Daniel Jacobowitz
first.

It would keep there the original synchronous readline() call being fully
backward compatible.


Regards,
Jan

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Fri, Mar 21, 2008 at 09:33:11PM +0100, Jan Kratochvil wrote:
> On Fri, 21 Mar 2008 20:01:51 +0100, Chet Ramey wrote:
> > I'm looking at the rl_unwind_protect thing a little more closely, and I'm
> > wondering why you didn't use rl_save_state and rl_restore_state
> [snip]
> 
> These functions are just not in the info document.
> They could be used to implement the GDB part of the patch
> 	http://sourceware.org/ml/gdb-patches/2007-06/txt00005.txt
> but it needs to undo the current async-readline patch from Daniel Jacobowitz
> first.
> 
> It would keep there the original synchronous readline() call being fully
> backward compatible.

Thanks.  I didn't know about those either.  Is there anything
currently broken with our readline wrapper that needs fixing, though?
If it's working, I would rather leave it alone.  We may in the future
want to really return to the main event loop while we're waiting for
the user to type, so that we can notice target events in async mode.
A blocking call to readline is a problem for that.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Jan Kratochvil]

On Fri, 21 Mar 2008 22:21:44 +0100, Daniel Jacobowitz wrote:
> On Fri, Mar 21, 2008 at 09:33:11PM +0100, Jan Kratochvil wrote:
> > On Fri, 21 Mar 2008 20:01:51 +0100, Chet Ramey wrote:
> > > I'm looking at the rl_unwind_protect thing a little more closely, and I'm
> > > wondering why you didn't use rl_save_state and rl_restore_state
...
> Thanks.  I didn't know about those either.  Is there anything
> currently broken with our readline wrapper that needs fixing, though?

I am not aware of any problem.


> If it's working, I would rather leave it alone.  We may in the future
> want to really return to the main event loop while we're waiting for
> the user to type, so that we can notice target events in async mode.
> A blocking call to readline is a problem for that.

OK, I was not aware of it.



Regards,
Jan

Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list

[Daniel Jacobowitz]

On Fri, Mar 21, 2008 at 10:26:09PM +0100, Jan Kratochvil wrote:
> > If it's working, I would rather leave it alone.  We may in the future
> > want to really return to the main event loop while we're waiting for
> > the user to type, so that we can notice target events in async mode.
> > A blocking call to readline is a problem for that.
> 
> OK, I was not aware of it.

OK, we'll leave it alone.  We might want to use rl_save_state and
rl_restore_state for something else, though.  Actually, we could
probably turn off readline's signal handling since we use it only
in callback mode...

-- 
Daniel Jacobowitz
CodeSourcery