Re: [PATCH] Cygwin: console: Fix regression in console input
Takashi Yano <[email protected]>
| Newsgroups | gmane.os.cygwin.patches |
|---|---|
| Message-ID | <[email protected]> |
Hi Johannes,
On Tue, 11 Aug 2026 12:34:19 +0200 (CEST)
Johannes Schindelin wrote:
> Hi Takashi,
>
> On Fri, 7 Aug 2026, Takashi Yano wrote:
>
> > On Fri, 7 Aug 2026 01:25:51 +0900 Takashi Yano wrote:
> > >
> > > On Thu, 6 Aug 2026 16:21:49 +0200 (CEST) Johannes Schindelin wrote:
> > > >
> > > > On Mon, 3 Aug 2026, Takashi Yano wrote:
> > > > [...]
> > > > > +
> > > > > for (i = 0; i < total_read; i ++)
> > > > > {
> > > > > DWORD nread = 1;
> > > > > @@ -1794,7 +1797,6 @@ fhandler_console::process_input_message (size_t len)
> > > > > }
> > > > >
> > > > > num_input_events_processed = i + 1;
> > > > > - num_chars += nread;
> > > > > if (toadd)
> > > > > {
> > > > > ssize_t ret;
> > > > > @@ -1813,21 +1815,17 @@ fhandler_console::process_input_message (size_t len)
> > > > > }
> > > > > }
> > > > > /* len == 0 if called from select.cc:peek_console() */
> > > > > - if (len && num_chars >= len)
> > > > > + if (input_ready && (len == 0 || con_ra.ralen >= len))
> > > >
> > > > Something GPT 5.6 Sol stumbled over, before noticing that this is a
> > > > pre-existing issue: the raw multi-pass caller passes full `buflen`
> > > > (not `buflen - copied_chars`). That was already there before this
> > > > patch (and both GPT and Opus think that this is incorrect, I haven't
> > > > had time to wrap my head around it), and the new `ralen`-based check
> > > > is actually neutral-to-better; and `tcflush(TCIFLUSH)` doesn't flush
> > > > `con_ra`.
> > > >
> > > > I'm fairly certain that this is out of scope for this here bug fix,
> > > > if it is an issue at all (what do you think? Is this `buflen` issue
> > > > real? Should `tcflush()` also flush `con_ra`?).
> > >
> > > Thanks for finding this. I rechecked here and concluded passing
> > > `buflen` for process_input_message() is correct. This is because, the
> > > length check in the process_input_message is:
> > >
> > > if (input_ready && (len == 0 || con_ra.ralen >= len))
> > > goto out;
> > >
> > > After the `while` loop, the chars in readahead data of length `con_ra.ralen`
> > > will be copied into buffer of `buflen`. So the `len` should be total buffer
> > > size here.
> >
> > `copied_chars` is always 0 here in the first place, isn't it?
>
> Not necessarily. The total-buffer-size explanation overlooks the
> `read_more` path. In non-canonical mode, `read()` jumps back there when
> fewer than `VMIN` bytes were copied, so a subsequent
> `process_input_message(buflen)` can run with `copied_chars` already
> greater than zero.
>
> For example, with `buflen == VMIN == 10`, the first pass may copy 6 bytes.
> On the second pass only 4 bytes of the callers buffer remain, but passing
> 10 can consume up to 10 more bytes into `con_ra`, leaving 6 in this
> processs private readahead. Passing `buflen - copied_chars` would stop
> after filling the remaining 4 bytes and leave later typeahead in the
> console buffer.
>
> The exact batching depends on `line_edit()`, but `copied_chars` is
> therefore not always zero at that call.
Ah, right. Thanks for letting me know that.
I'll submit another patch to fix the argument of process_input_message().
--
Takashi Yano <[email protected]>