From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9312 invoked by alias); 3 Jan 2007 21:46:49 -0000 Received: (qmail 9292 invoked by uid 22791); 3 Jan 2007 21:46:48 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Wed, 03 Jan 2007 21:46:43 +0000 Received: from drow by nevyn.them.org with local (Exim 4.63) (envelope-from ) id 1H2DwW-0006fP-AE; Wed, 03 Jan 2007 16:46:40 -0500 Date: Wed, 03 Jan 2007 21:46:00 -0000 From: Daniel Jacobowitz To: Jan Kratochvil Cc: gdb-patches@sources.redhat.com, "H. J. Lu" Subject: Re: PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list Message-ID: <20070103214640.GI17935@nevyn.them.org> Mail-Followup-To: Jan Kratochvil , gdb-patches@sources.redhat.com, "H. J. Lu" References: <20061128164658.GB20882@nevyn.them.org> <20061128165844.GA13667@lucon.org> <20061202184344.GA2197@lucon.org> <4571CF2A.3040608@case.edu> <20061202221541.GA9776@lucon.org> <45725FC9.9070304@case.edu> <20061217234530.GA20773@host0.dyn.jankratochvil.net> <4586F56F.5040304@case.edu> <20061219231926.GA20632@host0.dyn.jankratochvil.net> <20061226060028.GA13727@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20061226060028.GA13727@host0.dyn.jankratochvil.net> User-Agent: Mutt/1.5.13 (2006-08-11) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-01/txt/msg00079.txt.bz2 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 Daniel Jacobowitz * 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 Daniel Jacobowitz * 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; } 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.