[PATCH] Cygwin: pty: Fix input charset for non-cygwin apps with disable_pcon.

[PATCH] Cygwin: pty: Fix input charset for non-cygwin apps with disable_pcon.

[Takashi Yano]

- If the non-cygwin apps is executed under pseudo console disabled,
  multibyte input for the apps are garbled. This patch fixes the
  issue.
---
 winsup/cygwin/fhandler_tty.cc | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 6de591d9b..afaa4546e 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -271,8 +271,17 @@ fhandler_pty_master::accept_input ()
   bytes_left = eat_readahead (-1);
 
   HANDLE write_to = get_output_handle ();
+  char *buf = NULL;
   if (to_be_read_from_pcon ())
-    write_to = to_slave;
+    {
+      write_to = to_slave;
+      size_t nlen;
+      buf = convert_mb_str (GetConsoleCP (), &nlen,
+			    get_ttyp ()->term_code_page,
+			    (const char *) p, bytes_left);
+      p = buf;
+      bytes_left = nlen;
+    }
 
   if (!bytes_left)
     {
@@ -305,6 +314,8 @@ fhandler_pty_master::accept_input ()
 	    }
 	}
     }
+  if (buf)
+    mb_str_free (buf);
 
   SetEvent (input_available_event);
   ReleaseMutex (input_mutex);
-- 
2.28.0

Re: [PATCH] Cygwin: pty: Fix input charset for non-cygwin apps with disable_pcon.

[Corinna Vinschen]

Hi Takashi,

On Sep  8 18:57, Takashi Yano via Cygwin-patches wrote:
> - If the non-cygwin apps is executed under pseudo console disabled,
>   multibyte input for the apps are garbled. This patch fixes the
>   issue.
> ---
>  winsup/cygwin/fhandler_tty.cc | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index 6de591d9b..afaa4546e 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -271,8 +271,17 @@ fhandler_pty_master::accept_input ()
>    bytes_left = eat_readahead (-1);
>  
>    HANDLE write_to = get_output_handle ();
> +  char *buf = NULL;
>    if (to_be_read_from_pcon ())
> -    write_to = to_slave;
> +    {
> +      write_to = to_slave;
> +      size_t nlen;
> +      buf = convert_mb_str (GetConsoleCP (), &nlen,
> +			    get_ttyp ()->term_code_page,
> +			    (const char *) p, bytes_left);
> +      p = buf;
> +      bytes_left = nlen;
> +    }

How big are chances that the string in p is larger than 32767 chars?

I'd like to see convert_mb_str use a tmp_pathbuf buffer instead of
calling HeapAlloc/HeapFree every time.  That also drops the mb_str_free
entirely.

Isn't there a problem anyway with calling convert_mb_str?  Consider
a write call which stops in the middle of a multibyte char, the
second half only sent with the next write call.  convert_mb_str
only allows to convert complete multibyte chars, and the caller does
not keep something like an mbstate_t around, which would allow
continuation of split multibyte chars.


Corinna

Re: [PATCH] Cygwin: pty: Fix input charset for non-cygwin apps with disable_pcon.

[Takashi Yano]

Hi Corinna,

On Tue, 8 Sep 2020 20:42:47 +0200
Corinna Vinschen wrote:
> Hi Takashi,
> 
> On Sep  8 18:57, Takashi Yano via Cygwin-patches wrote:
> > - If the non-cygwin apps is executed under pseudo console disabled,
> >   multibyte input for the apps are garbled. This patch fixes the
> >   issue.
> > ---
> >  winsup/cygwin/fhandler_tty.cc | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> > index 6de591d9b..afaa4546e 100644
> > --- a/winsup/cygwin/fhandler_tty.cc
> > +++ b/winsup/cygwin/fhandler_tty.cc
> > @@ -271,8 +271,17 @@ fhandler_pty_master::accept_input ()
> >    bytes_left = eat_readahead (-1);
> >  
> >    HANDLE write_to = get_output_handle ();
> > +  char *buf = NULL;
> >    if (to_be_read_from_pcon ())
> > -    write_to = to_slave;
> > +    {
> > +      write_to = to_slave;
> > +      size_t nlen;
> > +      buf = convert_mb_str (GetConsoleCP (), &nlen,
> > +			    get_ttyp ()->term_code_page,
> > +			    (const char *) p, bytes_left);
> > +      p = buf;
> > +      bytes_left = nlen;
> > +    }
> 
> How big are chances that the string in p is larger than 32767 chars?
> 
> I'd like to see convert_mb_str use a tmp_pathbuf buffer instead of
> calling HeapAlloc/HeapFree every time.  That also drops the mb_str_free
> entirely.
> 
> Isn't there a problem anyway with calling convert_mb_str?  Consider
> a write call which stops in the middle of a multibyte char, the
> second half only sent with the next write call.  convert_mb_str
> only allows to convert complete multibyte chars, and the caller does
> not keep something like an mbstate_t around, which would allow
> continuation of split multibyte chars.

Thanks for the advice. I will submit a series of patches which
reflect your advice.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>