Re: [PATCH] Cygwin: console: Fix handling of surrogate pairs

On Thu, 28 May 2026 15:33:24 +0200 (CEST)
Johannes Schindelin wrote:
> Hi Takashi,
> 
> On Tue, 26 May 2026, Takashi Yano wrote:
> 
> > The commit 782aac590af7 introduced surrogate-pair handling. However,
> > it does not work as expected in the legacy console. This is because
> > a KeyDown event for ALT key with UnicodeChar == 0 is inserted the
> > between surrogate pair. The current code reads the next key event
> > unconditionally for the second UnicodeChar, but it is not correct.
> > This patch searches the next appropriate key event with a valid
> > UnicodeChar, ensuring that the second code unit is valid.
> > 
> > Fixes: 782aac590af7 ("Cygwin: console: Handle Unicode surrogate pairs.")
> > Signed-off-by: Takashi Yano <[email protected]>
> > Reviewed-by:
> > ---
> >  winsup/cygwin/fhandler/console.cc | 20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> > index 6fd4cd965..45eff6efe 100644
> > --- a/winsup/cygwin/fhandler/console.cc
> > +++ b/winsup/cygwin/fhandler/console.cc
> > @@ -1452,9 +1452,21 @@ fhandler_console::process_input_message (void)
> >  	    }
> >  	  else
> >  	    {
> > -	      WCHAR second = unicode_char >= 0xd800 && unicode_char <= 0xdbff
> > -		  && i + 1 < total_read ?
> > -		  input_rec[i + 1].Event.KeyEvent.uChar.UnicodeChar : 0;
> > +	      WCHAR second = 0;
> > +	      DWORD second_pos = i;
> > +	      if (unicode_char >= 0xd800 && unicode_char <= 0xdbff)
> > +		for (DWORD j = i + 1; j < total_read; j++)
> > +		  {
> > +		    /* Do not check bKeyDown. bKeyDown is 0 for surrogate
> > +		       pair in legacy console */
> > +		    if (input_rec[j].EventType == KEY_EVENT &&
> > +			input_rec[j].Event.KeyEvent.uChar.UnicodeChar)
> > +		      {
> > +			second = input_rec[j].Event.KeyEvent.uChar.UnicodeChar;
> > +			second_pos = j;
> > +			break;
> > +		      }
> > +		  }
> >  
> >  	      if (second < 0xdc00 || second > 0xdfff)
> >  		{
> > @@ -1465,7 +1477,7 @@ fhandler_console::process_input_message (void)
> 
> It's a bit unfortunate that the diff here hides the fact that the
> following is in the `else` branch...
> 
> In any case, the patch makes sense to me, and I really appreciate the
> commit message that puts the diff into context.

Thanks for reviewing. Pushed.

-- 
Takashi Yano <[email protected]>