Re: Patch: RFA: operate-and-get-next fix

Elena Zannoni
2002-07-24T16:22:57Z

Tom Tromey writes:
> >>>>> "Elena" == Elena Zannoni <ezannoni@redhat.com> writes:
>
> Elena> Could you come up with a testcase to add to the testsuite?
>
> Here is an updated version of the patch.
> It includes a new file, readline.exp. Currently this file contains
> two tests for operate-and-get-next.
>
> Is this ok to commit?
>
> Tom
>
>
> Index: ChangeLog
> from Tom Tromey <tromey@redhat.com>
>
> * defs.h (gdb_readline_wrapper): Declare.
> * utils.c (prompt_for_continue): Use gdb_readline_wrapper.
> * tracepoint.c (read_actions): Use gdb_readline_wrapper.
> * top.c (gdb_readline_wrapper): New function.
> (command_line_input): Use it.
>
> Index: defs.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/defs.h,v
> retrieving revision 1.91
> diff -u -r1.91 defs.h
> --- defs.h 21 Jun 2002 23:48:40 -0000 1.91
> +++ defs.h 23 Jul 2002 23:50:46 -0000
> @@ -535,6 +535,8 @@
>
> extern char *gdb_readline (char *);
>
> +extern char *gdb_readline_wrapper (char *);
> +
> extern char *command_line_input (char *, int, char *);
>
> extern void print_prompt (void);
> Index: top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/top.c,v
> retrieving revision 1.64
> diff -u -r1.64 top.c
> --- top.c 11 Jul 2002 13:50:49 -0000 1.64
> +++ top.c 23 Jul 2002 23:50:48 -0000
> @@ -947,6 +947,26 @@
> static int history_size;
> static char *history_filename;
>
> +/* This is like readline(), but it has some gdb-specific behavior. In
> + particular, if the user is in the middle of an
> + operate-and-get-next, we shuffle the hooks around so that the
> + expected result occurs. This is ugly, but it is inevitable given
> + that gdb switches between the two modes (async and not) of using
> + readline and that rl_pre_input_hook doesn't work properly in async
> + mode. */
> +char *
> +gdb_readline_wrapper (char *prompt)
> +{
> + /* Set the hook that works in this case. */
> + if (event_loop_p && after_char_processing_hook)
> + {
> + rl_pre_input_hook = (Function *) after_char_processing_hook;
> + after_char_processing_hook = NULL;
> + }
> +
> + return readline (prompt);
> +}
> +

Ok for the other changes, but I don't understand why you are setting
rl_pre_input_hook in the async case, while in top.c there is a comment
saying the opposite:

/* This hook only works correctly when we are using the
synchronous readline. */
rl_pre_input_hook = (Function *) gdb_rl_operate_and_get_next_completion;

The testfile looks ok too.

Elena

> \f
> #ifdef STOP_SIGNAL
> static void
> @@ -1174,7 +1194,7 @@
> }
> else if (command_editing_p && instream == stdin && ISATTY (instream))
> {
> - rl = readline (local_prompt);
> + rl = gdb_readline_wrapper (local_prompt);
> }
> else
> {
> Index: tracepoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/tracepoint.c,v
> retrieving revision 1.39
> diff -u -r1.39 tracepoint.c
> --- tracepoint.c 12 May 2002 04:20:06 -0000 1.39
> +++ tracepoint.c 23 Jul 2002 23:50:50 -0000
> @@ -854,7 +854,7 @@
> line = (*readline_hook) (prompt);
> else if (instream == stdin && ISATTY (instream))
> {
> - line = readline (prompt);
> + line = gdb_readline_wrapper (prompt);
> if (line && *line) /* add it to command history */
> add_history (line);
> }
> Index: utils.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/utils.c,v
> retrieving revision 1.72
> diff -u -r1.72 utils.c
> --- utils.c 5 Apr 2002 16:39:11 -0000 1.72
> +++ utils.c 23 Jul 2002 23:50:52 -0000
> @@ -1603,7 +1603,7 @@
> /* Call readline, not gdb_readline, because GO32 readline handles control-C
> whereas control-C to gdb_readline will cause the user to get dumped
> out to DOS. */
> - ignore = readline (cont_prompt);
> + ignore = gdb_readline_wrapper (cont_prompt);
>
> if (annotation_level > 1)
> printf_unfiltered ("\n\032\032post-prompt-for-continue\n");
> Index: testsuite/ChangeLog
> from Tom Tromey <tromey@redhat.com>
>
> * gdb.base/readline.exp: New file.
>
> Index: testsuite/gdb.base/readline.exp
> ===================================================================
> RCS file: testsuite/gdb.base/readline.exp
> diff -N testsuite/gdb.base/readline.exp
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.base/readline.exp 23 Jul 2002 23:51:02 -0000
> @@ -0,0 +1,190 @@
> +# Copyright 2002 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
> +
> +# This file was written by Tom Tromey <tromey@redhat.com>
> +
> +# This file is part of the gdb testsuite.
> +
> +#
> +# Tests for readline operations.
> +#
> +
> +# This function is used to test operate-and-get-next.
> +# NAME is the name of the test.
> +# ARGS is a list of alternating commands and expected results.
> +proc operate_and_get_next {name args} {
> + global gdb_prompt
> +
> + set my_gdb_prompt "($gdb_prompt| >)"
> +
> + set reverse {}
> + foreach {item result} $args {
> + verbose "sending $item"
> + sleep 1
> +
> + # We can't use gdb_test here because we might see a " >" prompt.
> + set status 0
> + send_gdb "$item\n"
> + gdb_expect {
> + -re "$item" {
> + # Ok
> + }
> + timeout {
> + set status 1
> + }
> + }
> +
> + if {! $status} {
> + gdb_expect {
> + -re "$result" {
> + # Ok.
> + }
> + timeout {
> + set status 1
> + }
> + }
> + }
> +
> + if {$status} {
> + fail "$name - send $item"
> + return 0
> + }
> + pass "$name - send $item"
> +
> + set reverse [linsert $reverse 0 $item $result]
> + }
> +
> + # Now use C-p to go back to the start.
> + foreach {item result} $reverse {
> + # Actually send C-p followed by C-l. This lets us recognize the
> + # command when gdb prints it again.
> + send_gdb "\x10\x0c"
> + set status 0
> + gdb_expect {
> + -re "$item" {
> + # Ok
> + }
> + timeout {
> + set status 1
> + }
> + }
> + if {$status} {
> + fail "$name - C-p to $item"
> + return 0
> + }
> + pass "$name - C-p to $item"
> + }
> +
> + # Now C-o through the list. Don't send the command, since it is
> + # already there. Strip off the first command from the list so we
> + # can see the next command inside the loop.
> + set count 0
> + foreach {item result} $args {
> + set status 0
> +
> + # If this isn't the first item, make sure we see the command at
> + # the prompt.
> + if {$count > 0} {
> + gdb_expect {
> + -re ".*$item" {
> + # Ok
> + }
> + timeout {
> + set status 1
> + }
> + }
> + }
> +
> + if {! $status} {
> + # For the last item, send a simple \n instead of C-o.
> + if {$count == [llength $args] - 2} {
> + send_gdb "\n"
> + } else {
> + # 15 is C-o.
> + send_gdb [format %c 15]
> + }
> + set status 0
> + gdb_expect {
> + -re "$result" {
> + # Ok
> + }
> + timeout {
> + set status 1
> + }
> + }
> + }
> +
> + if {$status} {
> + fail "$name - C-o for $item"
> + return 0
> + }
> + pass "$name - C-o for $item"
> +
> + set count [expr {$count + 2}]
> + }
> +
> + return 1
> +}
> +
> +
> +if $tracelevel {
> + strace $tracelevel
> +}
> +
> +# Don't let a .inputrc file or an existing setting of INPUTRC mess up
> +# the test results. Even if /dev/null doesn't exist on the particular
> +# platform, the readline library will use the default setting just by
> +# failing to open the file. OTOH, opening /dev/null successfully will
> +# also result in the default settings being used since nothing will be
> +# read from this file.
> +global env
> +if [info exists env(INPUTRC)] {
> + set old_inputrc $env(INPUTRC)
> +}
> +set env(INPUTRC) "/dev/null"
> +
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +
> +set oldtimeout1 $timeout
> +set timeout 30
> +
> +
> +# A simple test of operate-and-get-next.
> +operate_and_get_next "Simple operate-and-get-next" \
> + "p 1" ".* = 1" \
> + "p 2" ".* = 2"\
> + "p 3" ".* = 3"
> +
> +# Test operate-and-get-next with a secondary prompt.
> +operate_and_get_next "operate-and-get-next with secondary prompt" \
> + "if 1 > 0" "" \
> + "p 5" "" \
> + "end" ".* = 5"
> +
> +
> +# Restore globals modified in this test...
> +if [info exists old_inputrc] {
> + set env(INPUTRC) $old_inputrc
> +} else {
> + unset env(INPUTRC)
> +}
> +set timeout $oldtimeout1
> +
> +return 0