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

[Elena Zannoni <ezannoni at redhat dot com>]

Tom Tromey writes:
 > This patch fixes a recently-discovered problem with operate-and-get-next.
 > 
 > The problem is that C-o doesn't work when entering breakpoint
 > commands.  The user expects this to work, since such commands are
 > entered into the readline history.  Also, this is a very useful place
 > for C-o to work.
 > 
 > Unfortunately, this patch is a bit ugly.  The problem here is that we
 > need to use one of two different hooks, depending on the way that
 > readline is called.  But we can't know that in advance, since some
 > commands will call readline() even when event_loop_p is true.
 > 
 > So we route all calls to readline() through a wrapper function that
 > rearranges the hook values if required.
 > 
 > A better fix would be to change readline so that rl_pre_input_hook
 > works correctly even in the event-loop mode.  I believe I mentioned
 > this during the discussions of the original operate-and-get-next patch.
 > (I definitely don't have the time to investigate doing that.  Last
 > time I looked at it but it seemed like a real quagmire.)
 > 
 > Built and tested on x86 Red Hat Linux 6.2.
 > I tried it using things like:
 > 
 >     if 2 > 1
 >     p 5
 >     end
 > 
 > Any comments?
 > 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.88
 > diff -u -r1.88 defs.h
 > --- defs.h 18 Apr 2002 18:08:59 -0000 1.88
 > +++ defs.h 24 Apr 2002 01:28:05 -0000
 > @@ -525,6 +525,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.61
 > diff -u -r1.61 top.c
 > --- top.c 28 Mar 2002 01:35:55 -0000 1.61
 > +++ top.c 24 Apr 2002 01:28:12 -0000
 > @@ -947,6 +947,28 @@
 >  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)
 > +{
 > +  char *result;
 > +
 > +  /* Set the hook that works in this case.  */
 > +  if (event_loop_p && after_char_processing_hook)
 > +    {
 > +      rl_pre_input_hook = after_char_processing_hook;
 > +      after_char_processing_hook = NULL;
 > +    }
 > +
 > +  result = readline (prompt);
 > +}
 > +


Sorry for the delay in looking at this.

Shouldn't gdb_readline_wrapper return the result?

This stuff is getting impossibly convoluted. :-(

Could you come up with a testcase to add to the testsuite?

Elena


 >  
 >  #ifdef STOP_SIGNAL
 >  static void
 > @@ -1174,7 +1196,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.36
 > diff -u -r1.36 tracepoint.c
 > --- tracepoint.c 27 Mar 2002 21:35:35 -0000 1.36
 > +++ tracepoint.c 24 Apr 2002 01:28:14 -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 24 Apr 2002 01:28:15 -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");