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

Hi Johannes,

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

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.

>   Reviewed-by: Johannes Schindelin <[email protected]>

Thanks!

> > diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/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;
> >  	}
> >  
> > +      num_input_events_processed = i + 1;
> 
> 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.

Initializer added to constructor.

> > @@ -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 = true;
> >  	  get_ttyp ()->output_stopped &= ~BY_VSTOP;
> >  	  continue;
> > +	case not_signalled_but_done:
> > +	  break;
> 
> Dropping `not_signalled_but_done` from the shared switch case also drops
> the clearing of `BY_VSTOP` on this path. Probably fine, since no cygwin
> signal is delivered here, but a sentence in the commit message would save
> future git-blame archaeology.

Your expectation is correct. I'll add the explanation to the commit message.
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.

So, I'd change here a bit:
@@ -525,10 +532,11 @@ fhandler_termios::line_edit (const char *rptr, size_t nrea
d, termios& ti,
       switch (process_sigs (c, get_ttyp (), this))
        {
        case signalled:
-       case not_signalled_but_done:
        case done_with_debugger:
          sawsig = true;
          get_ttyp ()->output_stopped &= ~BY_VSTOP;
+         fallthrough;
+       case not_signalled_but_done:
          continue;
        case not_signalled_with_nat_reader:
          disable_eof_key = true;

and also change here:
@@ -466,7 +473,7 @@ not_a_sig:
          fh->discard_input ();
        }
       ti.c_lflag &= ~FLUSHO;
-      return not_signalled_but_done;
+      return need_send_sig ? not_signalled : not_signalled_but_done;
     }
   bool to_nat = !cyg_reader && pg_with_nat;
   return to_nat ? not_signalled_with_nat_reader : not_signalled;

I think this is more appropriate.

With the minor fixes above, I'd push this patch to master and
cygwin-3_6-branch.

-- 
Takashi Yano <[email protected]>