[PATCH] Be truthful about reporting whether readahead is available

[PATCH] Be truthful about reporting whether readahead is available

[Johannes Schindelin]

In 7346568 (Make requested console reports work, 2016-03-16), code was
introduced to report the current cursor position. It works by using a
pointer that either points to the next byte in the readahead buffer, or
to a NUL byte if the buffer is depleted, or the pointer is NULL.

These conditions are heeded in the fhandler_console::read() method, but
the condition that the pointer can point at the end of the readahead
buffer was not handled properly in the get_cons_readahead_valid()
method.

This poses a problem e.g. in Git for Windows (which uses a slightly
modified MSYS2 runtime which is in turn a slightly modified Cygwin
runtime) when vim queries the cursor position and immediately goes on to
read console input, erroneously thinking that the readahead buffer is
valid when it is already depleted instead. This condition results in an
apparent freeze that can be helped only by pressing keys repeatedly.

The full Git for Windows bug report is here:

	https://github.com/git-for-windows/git/issues/711

Let's just teach the get_cons_readahead_valid() method to handle a
depleted readahead buffer correctly.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/fhandler.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 4610557..bd1a923 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1453,7 +1453,8 @@ private:
   bool focus_aware () {return shared_console_info->con.use_focus;}
   bool get_cons_readahead_valid ()
   {
-    return shared_console_info->con.cons_rapoi != NULL;
+    return shared_console_info->con.cons_rapoi != NULL &&
+      *shared_console_info->con.cons_rapoi;
   }
 
   select_record *select_read (select_stuff *);
-- 
2.8.0.windows.1

Re: [PATCH] Be truthful about reporting whether readahead is available

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2179 bytes --]

Thomas?

Any input?

On Apr  5 10:52, Johannes Schindelin wrote:
> In 7346568 (Make requested console reports work, 2016-03-16), code was
> introduced to report the current cursor position. It works by using a
> pointer that either points to the next byte in the readahead buffer, or
> to a NUL byte if the buffer is depleted, or the pointer is NULL.
> 
> These conditions are heeded in the fhandler_console::read() method, but
> the condition that the pointer can point at the end of the readahead
> buffer was not handled properly in the get_cons_readahead_valid()
> method.
> 
> This poses a problem e.g. in Git for Windows (which uses a slightly
> modified MSYS2 runtime which is in turn a slightly modified Cygwin
> runtime) when vim queries the cursor position and immediately goes on to
> read console input, erroneously thinking that the readahead buffer is
> valid when it is already depleted instead. This condition results in an
> apparent freeze that can be helped only by pressing keys repeatedly.
> 
> The full Git for Windows bug report is here:
> 
> 	https://github.com/git-for-windows/git/issues/711
> 
> Let's just teach the get_cons_readahead_valid() method to handle a
> depleted readahead buffer correctly.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  winsup/cygwin/fhandler.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
> index 4610557..bd1a923 100644
> --- a/winsup/cygwin/fhandler.h
> +++ b/winsup/cygwin/fhandler.h
> @@ -1453,7 +1453,8 @@ private:
>    bool focus_aware () {return shared_console_info->con.use_focus;}
>    bool get_cons_readahead_valid ()
>    {
> -    return shared_console_info->con.cons_rapoi != NULL;
> +    return shared_console_info->con.cons_rapoi != NULL &&
> +      *shared_console_info->con.cons_rapoi;
>    }
>  
>    select_record *select_read (select_stuff *);
> -- 
> 2.8.0.windows.1


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Fwd: Re: [PATCH] Be truthful about reporting whether readahead is available

[Thomas Wolff]

>> Von: Corinna Vinschen <corinna-cygwin@cygwin.com>
>> An: cygwin-patches@cygwin.com
>> Cc: Thomas Wolff <towo@towo.net>
>> Datum: 5. April 2016 um 15:55
>> Betreff: Re: [PATCH] Be truthful about reporting whether readahead is 
>> available
>>
>> Thomas?
>>
>> Any input?
>>
Yes, let's fix the patch so. Sorry for the flaw.
Thomas

>>
>> On Apr 5 10:52, Johannes Schindelin wrote:
>>
>>> In 7346568 (Make requested console reports work, 2016-03-16), code was
>>> introduced to report the current cursor position. It works by using a
>>> pointer that either points to the next byte in the readahead buffer, or
>>> to a NUL byte if the buffer is depleted, or the pointer is NULL.
>>>
>>> These conditions are heeded in the fhandler_console::read() method, but
>>> the condition that the pointer can point at the end of the readahead
>>> buffer was not handled properly in the get_cons_readahead_valid()
>>> method.
>>>
>>> This poses a problem e.g. in Git for Windows (which uses a slightly
>>> modified MSYS2 runtime which is in turn a slightly modified Cygwin
>>> runtime) when vim queries the cursor position and immediately goes on to
>>> read console input, erroneously thinking that the readahead buffer is
>>> valid when it is already depleted instead. This condition results in an
>>> apparent freeze that can be helped only by pressing keys repeatedly.
>>>
>>> The full Git for Windows bug report is here:
>>>
>>> https://github.com/git-for-windows/git/issues/711
>>>
>>> Let's just teach the get_cons_readahead_valid() method to handle a
>>> depleted readahead buffer correctly.
>>>
>>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>>> ---
>>> winsup/cygwin/fhandler.h | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
>>> index 4610557..bd1a923 100644
>>> --- a/winsup/cygwin/fhandler.h
>>> +++ b/winsup/cygwin/fhandler.h
>>> @@ -1453,7 +1453,8 @@ private:
>>> bool focus_aware () {return shared_console_info->con.use_focus;}
>>> bool get_cons_readahead_valid ()
>>> {
>>> - return shared_console_info->con.cons_rapoi != NULL;
>>> + return shared_console_info->con.cons_rapoi != NULL &&
>>> + *shared_console_info->con.cons_rapoi;
>>> }
>>>
>>> select_record *select_read (select_stuff *);
>>> -- 
>>> 2.8.0.windows.1
>>>
>>
>> Thanks,
>> Corinna
>>
>> -- 
>> Corinna Vinschen Please, send mails regarding Cygwin to
>> Cygwin Maintainer cygwin AT cygwin DOT com
>> Red Hat
>>

Re: Fwd: Re: [PATCH] Be truthful about reporting whether readahead is available

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1929 bytes --]

On Apr  5 18:50, Thomas Wolff wrote:
> 
> >>Von: Corinna Vinschen <corinna-cygwin@cygwin.com>
> >>An: cygwin-patches@cygwin.com
> >>Cc: Thomas Wolff <towo@towo.net>
> >>Datum: 5. April 2016 um 15:55
> >>Betreff: Re: [PATCH] Be truthful about reporting whether readahead is
> >>available
> >>
> >>Thomas?
> >>
> >>Any input?
> >>
> Yes, let's fix the patch so. Sorry for the flaw.

No worries.

> Thomas
> 
> >>
> >>On Apr 5 10:52, Johannes Schindelin wrote:
> >>
> >>>In 7346568 (Make requested console reports work, 2016-03-16), code was
> >>>introduced to report the current cursor position. It works by using a
> >>>pointer that either points to the next byte in the readahead buffer, or
> >>>to a NUL byte if the buffer is depleted, or the pointer is NULL.
> >>>
> >>>These conditions are heeded in the fhandler_console::read() method, but
> >>>the condition that the pointer can point at the end of the readahead
> >>>buffer was not handled properly in the get_cons_readahead_valid()
> >>>method.
> >>>
> >>>This poses a problem e.g. in Git for Windows (which uses a slightly
> >>>modified MSYS2 runtime which is in turn a slightly modified Cygwin
> >>>runtime) when vim queries the cursor position and immediately goes on to
> >>>read console input, erroneously thinking that the readahead buffer is
> >>>valid when it is already depleted instead. This condition results in an
> >>>apparent freeze that can be helped only by pressing keys repeatedly.
> >>>
> >>>The full Git for Windows bug report is here:
> >>>
> >>>https://github.com/git-for-windows/git/issues/711
> >>>
> >>>Let's just teach the get_cons_readahead_valid() method to handle a
> >>>depleted readahead buffer correctly.

Patch applied.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Fwd: Re: [PATCH] Be truthful about reporting whether readahead is available

[Johannes Schindelin]

Hi Thomas,

On Tue, 5 Apr 2016, Thomas Wolff wrote:

> > > Betreff: Re: [PATCH] Be truthful about reporting whether readahead is
> > > available
> > >
> > > Thomas?
> > >
> > > Any input?
> > >
> Yes, let's fix the patch so. Sorry for the flaw.

No big deal, thanks for the ACK!

Ciao,
Johannes