[PATCH] Cygwin: console: Share readahead buffer within the same process.

[Corinna Vinschen]

Hi Takashi,

On Jan 25 18:45, Takashi Yano wrote:
> - The cause of the problem reported in
>   https://www.cygwin.com/ml/cygwin/2020-01/msg00220.html is that the
>   chars input before dup() cannot be read from the new file descriptor.
>   This is because the readahead buffer (rabuf) in the console is newly
>   created by dup(), and does not inherit from the parent. This patch
>   fixes the issue.
> ---
>  winsup/cygwin/fhandler.cc         | 58 ++++++++++++++++---------------
>  winsup/cygwin/fhandler.h          | 24 ++++++++-----
>  winsup/cygwin/fhandler_console.cc | 16 ++++++++-
>  winsup/cygwin/fhandler_termios.cc | 35 ++++++++++---------
>  winsup/cygwin/fhandler_tty.cc     |  2 +-
>  5 files changed, 80 insertions(+), 55 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
> index aeee8fe4d..ad4a7e61c 100644
> --- a/winsup/cygwin/fhandler.cc
> +++ b/winsup/cygwin/fhandler.cc
> @@ -44,11 +44,12 @@ void
>  fhandler_base::reset (const fhandler_base *from)
>  {
>    pc << from->pc;
> -  rabuf = NULL;
> -  ralen = 0;
> -  raixget = 0;
> -  raixput = 0;
> -  rabuflen = 0;
> +  ra.rabuf = NULL;
> +  ra.ralen = 0;
> +  ra.raixget = 0;
> +  ra.raixput = 0;
> +  ra.rabuflen = 0;
> +  set_rabuf ();
>    _refcnt = 0;
>  }
>  
> @@ -66,15 +67,15 @@ int
>  fhandler_base::put_readahead (char value)
>  {
>    char *newrabuf;
> -  if (raixput < rabuflen)
> +  if (raptr->raixput < raptr->rabuflen)
>      /* Nothing to do */;

This adds extra pointer access to critical code paths, even if only
in O_TEXT scenarios.  May I suggest dropping the extra pointer and
converting readahead access to access methods, kind of like this:

  class fhandler {
    char *&rabuf () { return ra.rabuf; }
    int &rabuflen () { return ra.rabuflen; }
    [...]

  class fhandler_console {
    char *&rabuf () { return con_ra.rabuf; }
    int &rabuflen () { return con_ra.rabuflen; }
    [...]

and then use those accessor methods throughout:

> -  else if ((newrabuf = (char *) realloc (rabuf, rabuflen += 32)))
> -    rabuf = newrabuf;

      else if ((newrabuf = (char *) realloc (rabuf (), rabuflen () += 32)))
        rabuf () = newrabuf;

etc.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20200127/4f780166/attachment.sig>