Hi Takashi,

On Thu, 9 Jul 2026, Takashi Yano wrote:

> On Wed, 8 Jul 2026 16:58:57 +0200 (CEST)
> Johannes Schindelin wrote:
> > 
> > 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.

Thank you for the clarification!

> 
> >   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.

Excellent!

Thank you,
Johannes

Reply via email to