Re: [PATCH v3] Cygwin: console: Correct previous NOFLSH fix

Hi Takashi,

On Thu, 9 Jul 2026, Takashi Yano wrote:

> On Wed, 8 Jul 2026 16:58:57 +0200 (CEST)
> Johannes Schindelin wrote:
> >=20
> > Thank you for v3. Both concerns from v2 are addressed, and the "stty i=
ntr
> > ^x; cat | non-cygwin-app" case now behaves as expected.
> >=20
> > I have two non-blocking observations further below, and a question: Ou=
t of
> > curiosity, not a blocker: how does the `with_debugger_nat` branch actu=
ally
> > get reached in practice? gdb normally reads the console only at its ow=
n
> > prompt, i.e. when the inferior is stopped and thus not foreground, so =
the
> > pre-conditions do not obviously line up.
>=20
> In console, this never happen. In pty, process_sigs() is called from
> pty master, so this is reached when non-cygwin inferior is foreground
> in gdb.

Thank you for the clarification!

>=20
> >   Reviewed-by: Johannes Schindelin <[email protected]>
>=20
> Thanks!
>=20
> > > diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhand=
ler/console.cc
> > > index 730bb0b45..cc4591c14 100644
> > > --- a/winsup/cygwin/fhandler/console.cc
> > > +++ b/winsup/cygwin/fhandler/console.cc
> > > @@ -1718,6 +1718,7 @@ fhandler_console::process_input_message (size_=
t len)
> > >  	  continue;
> > >  	}
> > > =20
> > > +      num_input_events_processed =3D i + 1;
> >=20
> > The new counter member is missing from the constructor's init list. It=
 is
> > safe in practice because `cnew()` zero-fills, but adding it explicitly
> > would match the surrounding style.
>=20
> Initializer added to constructor.
>=20
> > > @@ -525,11 +532,12 @@ fhandler_termios::line_edit (const char *rptr,=
 size_t nread, termios& ti,
> > >        switch (process_sigs (c, get_ttyp (), this))
> > >  	{
> > >  	case signalled:
> > > -	case not_signalled_but_done:
> > >  	case done_with_debugger:
> > >  	  sawsig =3D true;
> > >  	  get_ttyp ()->output_stopped &=3D ~BY_VSTOP;
> > >  	  continue;
> > > +	case not_signalled_but_done:
> > > +	  break;
> >=20
> > Dropping `not_signalled_but_done` from the shared switch case also dro=
ps
> > the clearing of `BY_VSTOP` on this path. Probably fine, since no cygwi=
n
> > signal is delivered here, but a sentence in the commit message would s=
ave
> > future git-blame archaeology.
>=20
> Your expectation is correct. I'll add the explanation to the commit mess=
age.
> In addition, the naming of return value 'not_signalled_but_done' means
> not_signalled && the processing for the key has been done. However, with
> this patch, the processing for the key is continued in the code below.
>=20
> So, I'd change here a bit:
> @@ -525,10 +532,11 @@ fhandler_termios::line_edit (const char *rptr, siz=
e_t nrea
> d, termios& ti,
>        switch (process_sigs (c, get_ttyp (), this))
>         {
>         case signalled:
> -       case not_signalled_but_done:
>         case done_with_debugger:
>           sawsig =3D true;
>           get_ttyp ()->output_stopped &=3D ~BY_VSTOP;
> +         fallthrough;
> +       case not_signalled_but_done:
>           continue;
>         case not_signalled_with_nat_reader:
>           disable_eof_key =3D true;
>=20
> and also change here:
> @@ -466,7 +473,7 @@ not_a_sig:
>           fh->discard_input ();
>         }
>        ti.c_lflag &=3D ~FLUSHO;
> -      return not_signalled_but_done;
> +      return need_send_sig ? not_signalled : not_signalled_but_done;
>      }
>    bool to_nat =3D !cyg_reader && pg_with_nat;
>    return to_nat ? not_signalled_with_nat_reader : not_signalled;
>=20
> I think this is more appropriate.
>=20
> With the minor fixes above, I'd push this patch to master and
> cygwin-3_6-branch.

Excellent!

Thank you,
Johannes