Re: [PATCH] Cygwin: pty: Treat CR/NL in accept_input() the same as in transfer_input()

Hi Mark,

Thanks for reviewing!

On Tue, 23 Jun 2026 00:41:11 -0700
Mark Geisert wrote:
> Hi Takashi,
> 
> On 6/12/2026 5:47 AM, Takashi Yano wrote:
> > In transfer_input(), CR and NL in the data transferred to nat-pipe
> > is treated as follows:
> >    1) If pseudo console is activated, convert NL to CR.
> >    2) If pseudo console is disabled, convert CR to NL.
> > This conversion is necessary to ensure non-cygwin apps can handle
> > CR/NL as expected. Therefor, CR and NL should be treated as the
> > same way in accept_input() if the data is sent to nat-pipe.
> 
> The above block is fine.
> 
> > Usually, when pseudo console is activated, the input data for non-
> > cygwin app is not treated by accept_input. However, accept_input()
> > handle the input data in pseudo console enabled mode, only in a
> > very short duration when pseudo console is about to setup, because
> > master::write() calls line_edit() in the pcon_start mode. If pseudo
> > console is disabled, accept_input() handles them, however usually
> > ICRNL flag is set, so line_edit() do this conversion. However, if
> > this flag is not set, the conversion added by this patch is needed
> > as well.
> 
> This block I'm having a bit of trouble to follow.  Can you possibly 
> reword to describe it in more orderly fashion?

What about:
  In the previous implementation, problems rarely occurred because
  accept_input() normally does not handle input for non-cygwin apps
  when the pseudo console is active. Under typical conditions, such
  input is snet to pseudo console directly by WriteFile(), so
  accept_input() is not involved and no onversion issues arise.

  There is, however, a brief period during pseudo console initialization
  in which accept_input *does* handle the input. This happens because
  master::write() invokes line_edit() while in pcons_start mode. During
  this short window, the input is processed in pseudo-console-enabled
  mode, and the usual conversion behaviour may not apply.

  When the pseudo console is disabled, accept_input() always handles
  the input, and in most cases the ICRNL flag is set by shell, so
  line_edit() performs the CR->NL conversion. But if the flag is not
  set, this conversion does not occur. Therefore, the additional
  conversion introduced by this patch is required to ensure consistent
  behaviour in both cases.
?

> > Fixes: f20641789427 ("Cygwin: pty: Reduce unecessary input transfer.")
> > Signed-off-by: Takashi Yano <[email protected]>
> > Reviewed-by:
> > ---
> >   winsup/cygwin/fhandler/pty.cc | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
> > index ef79ea679..30918c2f3 100644
> > --- a/winsup/cygwin/fhandler/pty.cc
> > +++ b/winsup/cygwin/fhandler/pty.cc
> > @@ -690,6 +690,14 @@ fhandler_pty_master::accept_input ()
> >   	  p = mbbuf;
> >   	  bytes_left = nlen;
> >   	}
> > +
> > +      char *p0 = p;
> > +      if (get_ttyp ()->pcon_activated)
> > +	while ((p0 = (char *) memchr (p0, '\n', bytes_left - (p0 - p))))
> > +	  *p0 = '\r';
> > +      else
> > +	while ((p0 = (char *) memchr (p0, '\r', bytes_left - (p0 - p))))
> > +	  *p0 = '\n';
> >       }
> >   
> >     if (!bytes_left)
> 
> The code of the patch looks LGTM.  Let me know what you think about my 
> comments when you can.

-- 
Takashi Yano <[email protected]>