[PATCH] Cygwin: pty: Improve CSI6n handling in pcon_start state

Thomas Wolff towo@towo.net
Sat Feb 28 13:19:30 GMT 2026


Am 27.02.2026 um 18:58 schrieb Johannes Schindelin:
> Hi Takashi,
>
> On Mon, 23 Feb 2026, Takashi Yano wrote:
>
>> Previsouly, CSI6n was not handled correctly if the some sequences
>> are appended after the responce for CSI6n. Especially, if the
>> appended sequence is a ESC sequence, which is longer than the
>> expected maximum length of the CSI6n responce, the sequence will
>> not be written atomically. With this patch, pcon_start state
>> is cleared at the end of CSI6n responce, and appended sequence
>> will be written outside of the CSI&n handling block.
I encounter stray CSI6n when I invoke wsl.exe via execl(p) within a 
forkpty child and I was puzzled why that occurs.
Could it be related to this issue (and hopefully fixed by the patch)?
Thomas

> The idea of breaking out of the CSI 6n loop at `R` and falling through to
> the normal write paths is sound, but I think the `towrite` accounting has
> a bug, and the commit message could use some work.
>
>> Fixes: f20641789427 ("Cygwin: pty: Reduce unecessary input transfer.")
>> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
>> Reviewed-by:
>> ---
>>   winsup/cygwin/fhandler/pty.cc | 20 +++++++++++++-------
>>   1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
>> index 838be4a2b..c1e03db41 100644
>> --- a/winsup/cygwin/fhandler/pty.cc
>> +++ b/winsup/cygwin/fhandler/pty.cc
>> @@ -2137,6 +2137,8 @@ fhandler_pty_master::close (int flag)
>>   ssize_t
>>   fhandler_pty_master::write (const void *ptr, size_t len)
>>   {
>> +  size_t towrite = len;
>> +
>>     ssize_t ret;
>>     char *p = (char *) ptr;
>>     termios &ti = tc ()->ti;
>> @@ -2171,6 +2173,8 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>>   	    }
>>   	  if (state == 1)
>>   	    {
>> +	      towrite--;
>> +	      ptr = p + i + 1;
> The per-byte `towrite--` only fires inside `state == 1`, so bytes before
> the ESC (which go through the `else` / `line_edit` branch) are never
> subtracted. If there happen to be N bytes before the ESC in the same
> write, `towrite` ends up N too large, and the `nat` pipe fast path reads
> past the end of the buffer.
>
> This can be fixed in a simpler way by not tracking `towrite` in the loop
> at all, and instead computing it once at the break, see below...
>
>>   	      if (ixput < wpbuf_len)
>>   		wpbuf[ixput++] = p[i];
>>   	      else
>> @@ -2184,7 +2188,10 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>>   	  else
>>   	    line_edit (p + i, 1, ti, &ret);
>>   	  if (state == 1 && p[i] == 'R')
>> -	    state = 2;
>> +	    {
>> +	      state = 2;
>> +	      break;
>> +	    }
>
> If you initialize `towrite = 0` and make this hunk look like this instead:
>
> 	  if (state == 1 && p[i] == 'R')
> -	    state = 2;
> +	    {
> +	      state = 2;
> +	      towrite = len - i - 1;
> +	      ptr = p + i + 1;
> +	      break;
> +	    }
>
> then no per-byte bookkeeping is needed, making the entire logic a lot more
> robust, not to mention: easier on the reader's brain.
>
> Regarding the commit message: beyond the typos ("Previsouly" =>
> "Previously", "responce" => "response" x3, "CSI&n" => "CSI 6n"), the body
> describes the bug as "the sequence will not be written atomically", but
> isn't the actual problem that bytes after the `R` terminator go through
> per-byte `line_edit` inside the CSI 6n loop and then hit `return len`
> without ever reaching the `nat` pipe fast path? In that case, it would be
> a routing problem, not an atomicity problem, and something like:
>
>      Cygwin: pty: Fix data after CSI 6n response bypassing normal write paths
>
>      When the terminal's CSI 6n response and subsequent data (e.g.
>      keystrokes) arrive in the same write buffer, `master::write()`
>      processes all of it inside the pcon_start loop and returns early.
>      Bytes after the 'R' terminator go through per-byte `line_edit()` in
>      that loop instead of falling through to the `nat` pipe fast path or
>      the normal bulk `line_edit()` call.
>
>      Fix this by breaking out of the loop when 'R' is found and letting the
>      remaining data fall through to the normal write paths, which are now
>      reachable because `pcon_start` has been cleared.
>
> would be potentially more accurate in describing what is going on.
>
> I am still quite fuzzy on the exact goings-on in the pty code, essentially
> cobbling it all together "on the side" because I unfortunately cannot
> afford to spend much focus on the Cygwin/MSYS2 runtime. Hopefully what I
> said above makes some sense?
>
> Ciao,
> Johannes
>
>>   	}
>>         if (state == 2)
>>   	{
>> @@ -2220,8 +2227,8 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>>   	    }
>>   	  get_ttyp ()->pcon_start_pid = 0;
>>   	}
>> -
>> -      return len;
>> +      if (towrite == 0)
>> +	return len;
>>       }
>>   
>>     /* Write terminal input to to_slave_nat pipe instead of output_handle
>> @@ -2233,15 +2240,14 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>>   	 is activated. */
>>         tmp_pathbuf tp;
>>         char *buf = (char *) ptr;
>> -      size_t nlen = len;
>> +      size_t nlen = towrite;
>>         if (get_ttyp ()->term_code_page != CP_UTF8)
>>   	{
>>   	  static mbstate_t mbp;
>>   	  buf = tp.c_get ();
>>   	  nlen = NT_MAX_PATH;
>> -	  convert_mb_str (CP_UTF8, buf, &nlen,
>> -			  get_ttyp ()->term_code_page, (const char *) ptr, len,
>> -			  &mbp);
>> +	  convert_mb_str (CP_UTF8, buf, &nlen, get_ttyp ()->term_code_page,
>> +			  (const char *) ptr, towrite, &mbp);
>>   	}
>>   
>>         for (size_t i = 0; i < nlen; i++)
>> -- 
>> 2.51.0
>>
>>



More information about the Cygwin-patches mailing list