Re: [PATCH v3 0/1] Cygwin: pty: Fix the behaviour of Ctrl-C in the pseudo console mode.

2019-09-13 Thread Takashi Yano
Hi Ken,

On Fri, 6 Sep 2019 17:59:02 +
Ken Brown wrote:
> P.S. I'm leaving tomorrow for a short vacation, so I might not have time to 
> review any more patches until I return in about a week.

I submitted five patches during your absence.

Four of them are for pseudo console support, the other one is for console.

[PATCH v5 0/1] Cygwin: pty: Fix the behaviour of Ctrl-C in the pseudo console 
mode.
[PATCH 0/1] Cygwin: pty: Fix screen alternation while pseudo console switching.
[PATCH 0/1] Cygwin: pty: Prevent the helper process from exiting by Ctrl-C
[PATCH v2 0/1] Cygwin: pty: Switch input and output pipes individually.
[PATCH 0/1] Cygwin: console: Fix read() in non-canonical mode.

Could you please review them?

-- 
Takashi Yano 


[PATCH v2 1/1] Cygwin: pty: Switch input and output pipes individually.

2019-09-13 Thread Takashi Yano
- Previously, input and output pipes were switched together between
  the traditional pty and the pseudo console. However, for example,
  if stdin is redirected to another device, it is better to leave
  input pipe traditional pty side even for non-cygwin program. This
  patch realizes such behaviour.
---
 winsup/cygwin/dtable.cc   |   6 +-
 winsup/cygwin/fhandler.h  |   9 +-
 winsup/cygwin/fhandler_console.cc |   7 +-
 winsup/cygwin/fhandler_tty.cc | 194 +-
 winsup/cygwin/select.cc   |   4 +-
 winsup/cygwin/spawn.cc|  44 +++
 winsup/cygwin/tty.cc  |   5 +-
 winsup/cygwin/tty.h   |   5 +-
 8 files changed, 173 insertions(+), 101 deletions(-)

diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 7b2e52005..cb5f47395 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -147,9 +147,9 @@ dtable::get_debugger_info ()
 void
 dtable::stdio_init ()
 {
-  int chk_order[] = {1, 0, 2};
   for (int i = 0; i < 3; i ++)
 {
+  const int chk_order[] = {1, 0, 2};
   int fd = chk_order[i];
   fhandler_base *fh = cygheap->fdtab[fd];
   if (fh && fh->get_major () == DEV_PTYS_MAJOR)
@@ -169,12 +169,14 @@ dtable::stdio_init ()
  FreeConsole ();
  if (AttachConsole (ptys->getHelperProcessId ()))
{
- ptys->fixup_after_attach (false);
+ ptys->fixup_after_attach (false, fd);
  break;
}
}
}
}
+  else if (fh && fh->get_major () == DEV_CONS_MAJOR)
+   break;
 }
 
   if (myself->cygstarted || ISSTATE (myself, PID_CYGPARENT))
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index e0c56cd34..1bf5dfb09 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2114,6 +2114,7 @@ class fhandler_pty_slave: public fhandler_pty_common
   HANDLE inuse;// used to indicate that a tty is in use
   HANDLE output_handle_cyg, io_handle_cyg;
   DWORD pid_restore;
+  int fd;
 
   /* Helper functions for fchmod and fchown. */
   bool fch_open_handles (bool chown);
@@ -2175,18 +2176,18 @@ class fhandler_pty_slave: public fhandler_pty_common
 copyto (fh);
 return fh;
   }
-  void set_switch_to_pcon (void);
+  void set_switch_to_pcon (int fd);
   void reset_switch_to_pcon (void);
   void push_to_pcon_screenbuffer (const char *ptr, size_t len);
-  void mask_switch_to_pcon (bool mask)
+  void mask_switch_to_pcon_in (bool mask)
   {
 if (!mask && get_ttyp ()->pcon_pid &&
get_ttyp ()->pcon_pid != myself->pid &&
kill (get_ttyp ()->pcon_pid, 0) == 0)
   return;
-get_ttyp ()->mask_switch_to_pcon = mask;
+get_ttyp ()->mask_switch_to_pcon_in = mask;
   }
-  void fixup_after_attach (bool native_maybe);
+  void fixup_after_attach (bool native_maybe, int fd);
   bool is_line_input (void)
   {
 return get_ttyp ()->ti.c_lflag & ICANON;
diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index 1b034f4b9..778279f99 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -3156,10 +3156,9 @@ fhandler_console::get_console_process_id (DWORD pid, 
bool match)
   tmp = 0;
   for (DWORD i=0; ihttps://github.com/microsoft/terminal/issues/95 */
+  tmp = list[i];
   HeapFree (GetProcessHeap (), 0, list);
   return tmp;
 }
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index b4591c17a..9aa832641 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -80,12 +80,13 @@ static void
 set_switch_to_pcon (void)
 {
   cygheap_fdenum cfd (false);
-  while (cfd.next () >= 0)
+  int fd;
+  while ((fd = cfd.next ()) >= 0)
 if (cfd->get_major () == DEV_PTYS_MAJOR)
   {
fhandler_base *fh = cfd;
fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
-   ptys->set_switch_to_pcon ();
+   ptys->set_switch_to_pcon (fd);
   }
 }
 
@@ -124,6 +125,29 @@ force_attach_to_pcon (HANDLE h)
break;
  }
  }
+   else if (cfd->get_major () == DEV_CONS_MAJOR)
+ {
+   fhandler_base *fh = cfd;
+   fhandler_console *cons = (fhandler_console *) fh;
+   if (n != 0
+   || h == cons->get_handle ()
+   || h == cons->get_output_handle ())
+ {
+   /* If the process is running on a console,
+  the parent process should be attached
+  to the same console. */
+   pinfo p (myself->ppid);
+   FreeConsole ();
+   if (AttachConsole (p->dwProcessId))
+ {
+   pcon_attached_to = -1;
+   attach_done = true;
+ }
+   else
+ pcon_attached_to = -1;
+   break;
+ }
+ 

[PATCH 1/1] Cygwin: console: Fix read() in non-canonical mode.

2019-09-13 Thread Takashi Yano
- In non-canonical mode, cygwin console returned only one character
  even if several keys are typed before read() called. This patch
  fixes this behaviour.
---
 winsup/cygwin/fhandler_console.cc | 606 --
 1 file changed, 315 insertions(+), 291 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index 778279f99..709b8255d 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -499,354 +499,378 @@ fhandler_console::process_input_message (void)
 
   termios *ti = &(get_ttyp ()->ti);
 
-  DWORD nread;
-  INPUT_RECORD input_rec;
-  const char *toadd = NULL;
+ /* Per MSDN, max size of buffer required is below 64K. */
+#define  INREC_SIZE(65536 / sizeof (INPUT_RECORD))
 
-  if (!ReadConsoleInputW (get_handle (), _rec, 1, ))
+  fhandler_console::input_states stat = input_processing;
+  DWORD total_read, i;
+  INPUT_RECORD input_rec[INREC_SIZE];
+
+  if (!PeekConsoleInputW (get_handle (), input_rec, INREC_SIZE, _read))
 {
-  termios_printf ("ReadConsoleInput failed, %E");
+  termios_printf ("PeekConsoleInput failed, %E");
   return input_error;
 }
 
-  const WCHAR _char = input_rec.Event.KeyEvent.uChar.UnicodeChar;
-  const DWORD _key_state = input_rec.Event.KeyEvent.dwControlKeyState;
-
-  /* check the event that occurred */
-  switch (input_rec.EventType)
+  for (i = 0; i < total_read; i ++)
 {
-case KEY_EVENT:
+  DWORD nread = 1;
+  const char *toadd = NULL;
 
-  con.nModifiers = 0;
+  const WCHAR _char =
+   input_rec[i].Event.KeyEvent.uChar.UnicodeChar;
+  const DWORD _key_state =
+   input_rec[i].Event.KeyEvent.dwControlKeyState;
 
-#ifdef DEBUGGING
-  /* allow manual switching to/from raw mode via ctrl-alt-scrolllock */
-  if (input_rec.Event.KeyEvent.bKeyDown
- && input_rec.Event.KeyEvent.wVirtualKeyCode == VK_SCROLL
- && (ctrl_key_state & (LEFT_ALT_PRESSED | LEFT_CTRL_PRESSED))
- == (LEFT_ALT_PRESSED | LEFT_CTRL_PRESSED))
+  /* check the event that occurred */
+  switch (input_rec[i].EventType)
{
- set_raw_win32_keyboard_mode (!con.raw_win32_keyboard_mode);
- return input_processing;
-   }
-#endif
+   case KEY_EVENT:
 
-  if (con.raw_win32_keyboard_mode)
-   {
- __small_sprintf (tmp, "\033{%u;%u;%u;%u;%u;%luK",
-  input_rec.Event.KeyEvent.bKeyDown,
-  input_rec.Event.KeyEvent.wRepeatCount,
-  input_rec.Event.KeyEvent.wVirtualKeyCode,
-  input_rec.Event.KeyEvent.wVirtualScanCode,
-  input_rec.Event.KeyEvent.uChar.UnicodeChar,
-  input_rec.Event.KeyEvent.dwControlKeyState);
- toadd = tmp;
- nread = strlen (toadd);
- break;
-   }
+ con.nModifiers = 0;
 
-  /* Ignore key up events, except for Alt+Numpad events. */
-  if (!input_rec.Event.KeyEvent.bKeyDown &&
- !is_alt_numpad_event (_rec))
-   return input_processing;
-  /* Ignore Alt+Numpad keys.  They are eventually handled below after
-releasing the Alt key. */
-  if (input_rec.Event.KeyEvent.bKeyDown
- && is_alt_numpad_key (_rec))
-   return input_processing;
-
-  if (ctrl_key_state & SHIFT_PRESSED)
-   con.nModifiers |= 1;
-  if (ctrl_key_state & RIGHT_ALT_PRESSED)
-   con.nModifiers |= 2;
-  if (ctrl_key_state & CTRL_PRESSED)
-   con.nModifiers |= 4;
-  if (ctrl_key_state & LEFT_ALT_PRESSED)
-   con.nModifiers |= 8;
-
-  /* Allow Backspace to emit ^? and escape sequences. */
-  if (input_rec.Event.KeyEvent.wVirtualKeyCode == VK_BACK)
-   {
- char c = con.backspace_keycode;
- nread = 0;
- if (ctrl_key_state & ALT_PRESSED)
+#ifdef DEBUGGING
+ /* allow manual switching to/from raw mode via ctrl-alt-scrolllock */
+ if (input_rec[i].Event.KeyEvent.bKeyDown
+ && input_rec[i].Event.KeyEvent.wVirtualKeyCode == VK_SCROLL
+ && (ctrl_key_state & (LEFT_ALT_PRESSED | LEFT_CTRL_PRESSED))
+ == (LEFT_ALT_PRESSED | LEFT_CTRL_PRESSED))
{
- if (con.metabit)
-   c |= 0x80;
- else
-   tmp[nread++] = '\e';
+ set_raw_win32_keyboard_mode (!con.raw_win32_keyboard_mode);
+ continue;
}
- tmp[nread++] = c;
- tmp[nread] = 0;
- toadd = tmp;
-   }
-  /* Allow Ctrl-Space to emit ^@ */
-  else if (input_rec.Event.KeyEvent.wVirtualKeyCode
-  == (wincap.has_con_24bit_colors () ? '2' : VK_SPACE)
-  && (ctrl_key_state & CTRL_PRESSED)
-  && !(ctrl_key_state & ALT_PRESSED))
-   toadd = "";
-  else if (unicode_char == 0
-  /* arrow/function keys */
-  || 

[PATCH 0/1] Cygwin: console: Fix read() in non-canonical mode.

2019-09-13 Thread Takashi Yano
- In non-canonical mode, cygwin console returned only one character
  even if several keys are typed before read() called. This patch
  fixes this behaviour.

Takashi Yano (1):
  Cygwin: console: Fix read() in non-canonical mode.

 winsup/cygwin/fhandler_console.cc | 606 --
 1 file changed, 315 insertions(+), 291 deletions(-)

-- 
2.21.0



[PATCH 0/1] Cygwin: pty: Switch input and output pipes individually.

2019-09-13 Thread Takashi Yano
- Previously, input and output pipes were switched together between
  the traditional pty and the pseudo console. However, for example,
  if stdin is redirected to another device, it is better to leave
  input pipe traditional pty side even for non-cygwin program. This
  patch realizes such behaviour.

Takashi Yano (1):
  Cygwin: pty: Switch input and output pipes individually.

 winsup/cygwin/dtable.cc   |   6 +-
 winsup/cygwin/fhandler.h  |   9 +-
 winsup/cygwin/fhandler_console.cc |   7 +-
 winsup/cygwin/fhandler_tty.cc | 256 --
 winsup/cygwin/select.cc   |   4 +-
 winsup/cygwin/spawn.cc|  44 +++--
 winsup/cygwin/tty.cc  |   5 +-
 winsup/cygwin/tty.h   |   5 +-
 8 files changed, 209 insertions(+), 127 deletions(-)

-- 
2.21.0



[PATCH 1/1] Cygwin: pty: Switch input and output pipes individually.

2019-09-13 Thread Takashi Yano
- Previously, input and output pipes were switched together between
  the traditional pty and the pseudo console. However, for example,
  if stdin is redirected to another device, it is better to leave
  input pipe traditional pty side even for non-cygwin program. This
  patch realizes such behaviour.
---
 winsup/cygwin/dtable.cc   |   6 +-
 winsup/cygwin/fhandler.h  |   9 +-
 winsup/cygwin/fhandler_console.cc |   7 +-
 winsup/cygwin/fhandler_tty.cc | 256 --
 winsup/cygwin/select.cc   |   4 +-
 winsup/cygwin/spawn.cc|  44 +++--
 winsup/cygwin/tty.cc  |   5 +-
 winsup/cygwin/tty.h   |   5 +-
 8 files changed, 209 insertions(+), 127 deletions(-)

diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 7b2e52005..cb5f47395 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -147,9 +147,9 @@ dtable::get_debugger_info ()
 void
 dtable::stdio_init ()
 {
-  int chk_order[] = {1, 0, 2};
   for (int i = 0; i < 3; i ++)
 {
+  const int chk_order[] = {1, 0, 2};
   int fd = chk_order[i];
   fhandler_base *fh = cygheap->fdtab[fd];
   if (fh && fh->get_major () == DEV_PTYS_MAJOR)
@@ -169,12 +169,14 @@ dtable::stdio_init ()
  FreeConsole ();
  if (AttachConsole (ptys->getHelperProcessId ()))
{
- ptys->fixup_after_attach (false);
+ ptys->fixup_after_attach (false, fd);
  break;
}
}
}
}
+  else if (fh && fh->get_major () == DEV_CONS_MAJOR)
+   break;
 }
 
   if (myself->cygstarted || ISSTATE (myself, PID_CYGPARENT))
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index e0c56cd34..1bf5dfb09 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2114,6 +2114,7 @@ class fhandler_pty_slave: public fhandler_pty_common
   HANDLE inuse;// used to indicate that a tty is in use
   HANDLE output_handle_cyg, io_handle_cyg;
   DWORD pid_restore;
+  int fd;
 
   /* Helper functions for fchmod and fchown. */
   bool fch_open_handles (bool chown);
@@ -2175,18 +2176,18 @@ class fhandler_pty_slave: public fhandler_pty_common
 copyto (fh);
 return fh;
   }
-  void set_switch_to_pcon (void);
+  void set_switch_to_pcon (int fd);
   void reset_switch_to_pcon (void);
   void push_to_pcon_screenbuffer (const char *ptr, size_t len);
-  void mask_switch_to_pcon (bool mask)
+  void mask_switch_to_pcon_in (bool mask)
   {
 if (!mask && get_ttyp ()->pcon_pid &&
get_ttyp ()->pcon_pid != myself->pid &&
kill (get_ttyp ()->pcon_pid, 0) == 0)
   return;
-get_ttyp ()->mask_switch_to_pcon = mask;
+get_ttyp ()->mask_switch_to_pcon_in = mask;
   }
-  void fixup_after_attach (bool native_maybe);
+  void fixup_after_attach (bool native_maybe, int fd);
   bool is_line_input (void)
   {
 return get_ttyp ()->ti.c_lflag & ICANON;
diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index 1b034f4b9..778279f99 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -3156,10 +3156,9 @@ fhandler_console::get_console_process_id (DWORD pid, 
bool match)
   tmp = 0;
   for (DWORD i=0; ihttps://github.com/microsoft/terminal/issues/95 */
+  tmp = list[i];
   HeapFree (GetProcessHeap (), 0, list);
   return tmp;
 }
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index b4591c17a..6a3dac7ae 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -80,12 +80,13 @@ static void
 set_switch_to_pcon (void)
 {
   cygheap_fdenum cfd (false);
-  while (cfd.next () >= 0)
+  int fd;
+  while ((fd = cfd.next ()) >= 0)
 if (cfd->get_major () == DEV_PTYS_MAJOR)
   {
fhandler_base *fh = cfd;
fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
-   ptys->set_switch_to_pcon ();
+   ptys->set_switch_to_pcon (fd);
   }
 }
 
@@ -97,33 +98,66 @@ force_attach_to_pcon (HANDLE h)
 {
   /* First time, attach to the pty whose handle value is match.
 Second time, try to attach to any pty. */
-  cygheap_fdenum cfd (false);
-  while (cfd.next () >= 0)
-   if (cfd->get_major () == DEV_PTYS_MAJOR)
- {
-   fhandler_base *fh = cfd;
-   fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
-   if (n != 0
-   || h == ptys->get_handle ()
-   || h == ptys->get_output_handle ())
- {
-   if (fhandler_console::get_console_process_id
- (ptys->getHelperProcessId (), true))
- attach_done = true;
-   else
- {
-   FreeConsole ();
-   if (AttachConsole (ptys->getHelperProcessId ()))
- {
-