public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Takashi Yano <tyan0@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin] Cygwin: console: Fix typeahead key swapping which still occurs.
Date: Fri, 18 Mar 2022 13:47:24 +0000 (GMT)	[thread overview]
Message-ID: <20220318134724.241CF3858D3C@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=fcb182387a23d59ad85896e9a92bab9bca37adec

commit fcb182387a23d59ad85896e9a92bab9bca37adec
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Mar 18 21:35:07 2022 +0900

    Cygwin: console: Fix typeahead key swapping which still occurs.
    
    - The commit "Cygwin: console: Improve the code to avoid typeahead
      key swapping." did not solve the problem enough. Two unexpected
      things happen.
      (1) wVirtualKeyCode and wVirtualScanCode of readback key event may
          be null'ed even if they are not zero on WriteConsoleInputW().
          Therefore, memcmp() may report the event sequence is not equal.
      (2) WriteConsoleInputW() may not be atomic. The event sequence
          which is written by WriteConsoleInputW() may be inserted by
          key input in the middle of the sequence. Current code gives
          up to fix in this situation.
      This patch should fix that issue.

Diff:
---
 winsup/cygwin/fhandler_console.cc | 58 +++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 241ca48ea..b92d758d1 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -180,6 +180,29 @@ cons_master_thread (VOID *arg)
   return 0;
 }
 
+/* Compare two INPUT_RECORD sequences */
+static inline bool
+inrec_eq (const INPUT_RECORD *a, const INPUT_RECORD *b, DWORD n)
+{
+  for (DWORD i = 0; i < n; i++)
+    {
+      if (a[i].EventType == KEY_EVENT && b[i].EventType == KEY_EVENT)
+	{ /* wVirtualKeyCode and wVirtualScanCode of the readback
+	     key event may be different from that of written event. */
+	  const KEY_EVENT_RECORD *ak = &a[i].Event.KeyEvent;
+	  const KEY_EVENT_RECORD *bk = &b[i].Event.KeyEvent;
+	  if (ak->bKeyDown != bk->bKeyDown
+	      || ak->uChar.UnicodeChar != bk->uChar.UnicodeChar
+	      || ak->dwControlKeyState != bk->dwControlKeyState
+	      || ak->wRepeatCount != bk->wRepeatCount)
+	    return false;
+	}
+      else if (memcmp (a + i, b + i, sizeof (INPUT_RECORD)) != 0)
+	return false;
+    }
+  return true;
+}
+
 /* This thread processes signals derived from input messages.
    Without this thread, those signals can be handled only when
    the process calls read() or select(). This thread reads input
@@ -328,30 +351,25 @@ remove_record:
 	      if (n < total_read)
 		break; /* Someone has read input without acquiring
 			  input_mutex. ConEmu cygwin-connector? */
-	      if (memcmp (input_rec, tmp, m::bytes (total_read)) == 0)
+	      if (inrec_eq (input_rec, tmp, total_read))
 		break; /* OK */
 	      /* Try to fix */
-	      DWORD incr = n - total_read;
-	      DWORD ofst;
-	      for (ofst = 1; ofst <= incr; ofst++)
-		if (memcmp (input_rec, tmp + ofst, m::bytes (total_read)) == 0)
+	      acquire_attach_mutex (mutex_timeout);
+	      ReadConsoleInputW (p->input_handle, tmp, inrec_size, &n);
+	      release_attach_mutex ();
+	      for (DWORD i = 0, j = 0; j < n; j++)
+		if (i == total_read || !inrec_eq (input_rec + i, tmp + j, 1))
 		  {
-		    acquire_attach_mutex (mutex_timeout);
-		    ReadConsoleInputW (p->input_handle, tmp, inrec_size, &n);
-		    release_attach_mutex ();
-		    memcpy (input_rec, tmp + ofst, m::bytes (total_read));
-		    memcpy (input_rec + total_read, tmp, m::bytes (ofst));
-		    if (n > ofst + total_read)
-		      memcpy (input_rec + total_read + ofst,
-			      tmp + ofst + total_read,
-			      m::bytes (n - (ofst + total_read)));
-		    total_read = n;
-		    break;
+		    if (total_read + j - i >= n)
+		      { /* Something is wrong. Giving up. */
+			WriteConsoleInputW (p->input_handle, tmp, n, &n);
+			goto skip_writeback;
+		      }
+		    input_rec[total_read + j - i] = tmp[j];
 		  }
-	      if (ofst > incr)
-		break; /* Writeback was not atomic. Or someone has read
-			  input without acquiring input_mutex.
-			  Giving up because hard to fix. */
+		else
+		  i++;
+	      total_read = n;
 	    }
 	  while (true);
 	}


                 reply	other threads:[~2022-03-18 13:47 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220318134724.241CF3858D3C@sourceware.org \
    --to=tyan0@sourceware.org \
    --cc=cygwin-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).