Re: Regression in history search in VI mode?

[Uwe Doering]

Hi Chet,

Chet Ramey wrote:
> Uwe Doering wrote:
>> Hi there,
>>
>> I found what I think is a regression in Bash 3.2PL25 vs. 3.1PL17.  In
>> VI mode, when I press ESC and '/' (or ESC and '?') in insert mode
>> Bash is supposed to switch to command mode and enter the history
>> search function.  This works well if there is a pause between ESC and
>> the next key.  However, if I hit the keys in quick succession I get a
>> beep instead of the expected '/' or '?' at the start of the history
>> search input line.  When I enter '/' or '?' again at this point it
>> does what I want.
>
> How quickly?  Even when I press ESC and `/' as close to simultaneously
> as possible, I can't reproduce this.  Has anyone else seen it?

I'd guess less than 250 ms apart or so.  This delay is probably caused 
by the X server or some other part of the system that has special 
handling for ESC sequences (arrow keys etc).  If the delay times out the 
ESC is sent to Bash as a solitary character, which makes a difference 
with regard to its own processing of ESC sequences.

But in any case, I found the cause of that strange effect by now (after 
countless hours of debugging).  From Bash 3.1 to 3.2 there was a change 
with regard to the wraparound of the 'push_index' and 'pop_index' 
pointers in 'lib/readline/input.c'.  This unfortunately broke the 
backward wraparound of 'pop_index' in _rl_unget_char().

In 3.1, 'pop_index' can be in a range between 0 and 510.  So in 
_rl_unget_char() the backward wraparound sequence

  if (pop_index < 0)
    pop_index = ibuffer_len - 1;

is okay.  'ibuffer_len' is 511, so we end up at offset 510.  However, in 
3.2 the range for 'pop_index' is 0 to 511.  So if we stuff one character 
back into the buffer via _rl_unget_char() we still store it at offset 
510.  This causes not only one subsequent read from that buffer but 
instead two of them (at offset 510 and 511) because the buffer is 
considered empty only if it wraps around to 0 again ('pop_index' gets 
compared to 'push_index' which is 0 initially).

Now, at offset 511 there is a NUL character (it's a statically defined 
buffer, so its contents is initially all zeros), which of course wreaks 
havoc with Bash's key processing.  That's where that strange effect came 
from.

The workaround I described earlier turned out to work only because that 
special key sequence advanced 'push_index' and 'pop_index' by 1, so 
there was no longer a backward wraparound involved and Bash started to 
work as expected.

As to the fact that you couldn't reproduce the problem at your end, I 
speculate that there is perhaps something in the initialization scripts 
of your Bash that advances said pointers already before you get the 
first prompt.  So no wraparound would happen.

Anyway, the fix for that bug is simple.  Just modify the lines mentioned 
above to read

  if (pop_index < 0)
    pop_index = ibuffer_len;

This lets 'pop_index' wrap around from 0 to 511, which is the right 
thing to do in Bash 3.2.  Right now I have a local patch in my copy of 
FreeBSD's Bash port, which I also attached to this mail for your 
convenience.  But I would of course be glad if this could be fixed by an 
official patch at some point in the future.

Regards,

   Uwe
--
Uwe Doering         |  EscapeBox - Managed On-Demand UNIX Servers
gemini@geminix.org  |  http://www.escapebox.net

--- lib/readline/input.c.orig   Sun Sep 23 01:47:46 2007
+++ lib/readline/input.c        Sun Sep 23 01:48:37 2007
@@ -154,7 +154,7 @@ _rl_unget_char (key)
     {
       pop_index--;
       if (pop_index < 0)
-       pop_index = ibuffer_len - 1;
+       pop_index = ibuffer_len;
       ibuffer[pop_index] = key;
       return (1);
     }