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

[Jan Kratochvil <jan.kratochvil@redhat.com>]

[-- 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"