[PATCH 0/4] Cygwin: console: Some fixes for console in xterm mode.

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/4] Cygwin: console: Some fixes for console in xterm mode.

Takashi Yano
Takashi Yano (4):
  Cygwin: console: Revise the code to fix tab position.
  Cygwin: console: Fix setting/unsetting xterm mode for input.
  Cygwin: console: Prevent buffer overrun.
  Cygwin: console: Add a workaround for "ESC 7" and "ESC 8".

 winsup/cygwin/fhandler.h          |  1 +
 winsup/cygwin/fhandler_console.cc | 91 ++++++++++++++++++-------------
 2 files changed, 55 insertions(+), 37 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/4] Cygwin: console: Revise the code to fix tab position.

Takashi Yano
- This patch fixes the issue that the cursor position is broken if
  window size is changed while executing vim, less etc.
---
 winsup/cygwin/fhandler_console.cc | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 64e12b832..7c97a7868 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -417,19 +417,10 @@ fhandler_console::set_cursor_maybe ()
 void
 fhandler_console::fix_tab_position (void)
 {
-  char buf[2048] = {0,};
-  /* Save cursor position */
-  __small_sprintf (buf+strlen (buf), "\0337");
-  /* Clear all horizontal tabs */
-  __small_sprintf (buf+strlen (buf), "\033[3g");
-  /* Set horizontal tabs */
-  for (int col=8; col<con.dwWinSize.X; col+=8)
-    __small_sprintf (buf+strlen (buf), "\033[%d;%dH\033H", 1, col+1);
-  /* Restore cursor position */
-  __small_sprintf (buf+strlen (buf), "\0338");
+  /* Re-setting ENABLE_VIRTUAL_TERMINAL_PROCESSING
+     fixes the tab position. */
+  request_xterm_mode_output (false);
   request_xterm_mode_output (true);
-  DWORD dwLen;
-  WriteConsole (get_output_handle (), buf, strlen (buf), &dwLen, 0);
 }
 
 bool
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/4] Cygwin: console: Fix setting/unsetting xterm mode for input.

Takashi Yano
In reply to this post by Takashi Yano
- This patch fixes the issue that xterm compatible mode for input
  is not correctly set/unset in some situation such as:
   1) cat is stopped by ctrl-c.
   2) The window size is changed in less.
  In case 1), request_xterm_mode_input(true) is called in read(),
  however, cat is stopped without request_xterm_mode_input(false).
  In case 2), less uses longjmp in signal handler, therefore,
  corresponding request_xterm_mode_input(false) is not called if
  the SIGWINCH signal is sent within read(). With this patch,
  InterlockedExchange() is used instead of InterlockedIncrement/
  Decrement().
---
 winsup/cygwin/fhandler_console.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 7c97a7868..9c5b80181 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -267,7 +267,7 @@ fhandler_console::request_xterm_mode_input (bool req)
     return;
   if (req)
     {
-      if (InterlockedIncrement (&con.xterm_mode_input) == 1)
+      if (InterlockedExchange (&con.xterm_mode_input, 1) == 0)
  {
   DWORD dwMode;
   GetConsoleMode (get_handle (), &dwMode);
@@ -277,7 +277,7 @@ fhandler_console::request_xterm_mode_input (bool req)
     }
   else
     {
-      if (InterlockedDecrement (&con.xterm_mode_input) == 0)
+      if (InterlockedExchange (&con.xterm_mode_input, 0) == 1)
  {
   DWORD dwMode;
   GetConsoleMode (get_handle (), &dwMode);
@@ -1171,6 +1171,7 @@ fhandler_console::close ()
       if ((NT_SUCCESS (status) && obi.HandleCount == 1)
   || myself->pid == con.owner)
  request_xterm_mode_output (false);
+      request_xterm_mode_input (false);
     }
 
   release_output_mutex ();
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/4] Cygwin: console: Prevent buffer overrun.

Takashi Yano
In reply to this post by Takashi Yano
- This patch prevent potential buffer overrun in the code handling
  escape sequences.
---
 winsup/cygwin/fhandler_console.cc | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 9c5b80181..8b4687724 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -3094,7 +3094,8 @@ fhandler_console::write (const void *vsrc, size_t len)
  case gotarg1:
   if (isdigit (*src))
     {
-      con.args[con.nargs] = con.args[con.nargs] * 10 + *src - '0';
+      if (con.nargs < MAXARGS)
+ con.args[con.nargs] = con.args[con.nargs] * 10 + *src - '0';
       wpbuf_put (*src);
       src++;
     }
@@ -3102,9 +3103,8 @@ fhandler_console::write (const void *vsrc, size_t len)
     {
       wpbuf_put (*src);
       src++;
-      con.nargs++;
-      if (con.nargs > MAXARGS)
- con.nargs--;
+      if (con.nargs < MAXARGS)
+ con.nargs++;
     }
   else if (*src == ' ')
     {
@@ -3117,9 +3117,8 @@ fhandler_console::write (const void *vsrc, size_t len)
     con.state = gotcommand;
   break;
  case gotcommand:
-  con.nargs ++;
-  if (con.nargs > MAXARGS)
-    con.nargs--;
+  if (con.nargs < MAXARGS)
+    con.nargs++;
   char_command (*src++);
   con.state = normal;
   wpixput = 0;
@@ -3183,9 +3182,8 @@ fhandler_console::write (const void *vsrc, size_t len)
     {
       con.state = gotarg1;
       wpbuf_put (*src);
-      con.nargs++;
-      if (con.nargs > MAXARGS)
- con.nargs--;
+      if (con.nargs < MAXARGS)
+ con.nargs++;
       src++;
     }
   else if (isalpha (*src))
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/4] Cygwin: console: Add a workaround for "ESC 7" and "ESC 8".

Takashi Yano
In reply to this post by Takashi Yano
- In xterm compatible mode, "ESC 7" and "ESC 8" do not work properly
  in the senario:
   1) Execute /bin/ls /bin to fill screen.
   2) Sned CSI?1049h to alternate screen.
   3) Reduce window size.
   4) Send CSI?1049l to resume screen.
   5) Send "ESC 7" and "ESC 8".
  After sending "ESC 8", the cursor goes to incorrect position. This
  patch adds a workaround for this issue.
---
 winsup/cygwin/fhandler.h          |  1 +
 winsup/cygwin/fhandler_console.cc | 53 +++++++++++++++++++++++--------
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index adaf19203..463bb83ab 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1869,6 +1869,7 @@ class dev_console
   bool alternate_charset_active;
   bool metabit;
   char backspace_keycode;
+  bool screen_alternated; /* For xterm compatible mode only */
 
   char my_title_buf [TITLESIZE + 1];
 
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 8b4687724..dffee240a 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -207,6 +207,8 @@ fhandler_console::setup ()
       con.dwLastCursorPosition.Y = -1;
       con.dwLastMousePosition.X = -1;
       con.dwLastMousePosition.Y = -1;
+      con.savex = con.savey = -1;
+      con.screen_alternated = false;
       con.dwLastButtonState = 0; /* none pressed */
       con.last_button_code = 3; /* released */
       con.underline_color = FOREGROUND_GREEN | FOREGROUND_BLUE;
@@ -2130,6 +2132,10 @@ fhandler_console::char_command (char c)
   break;
  case 'h': /* DECSET */
  case 'l': /* DECRST */
+  if (c == 'h')
+    con.screen_alternated = true;
+  else
+    con.screen_alternated = false;
   wpbuf_put (c);
   /* Just send the sequence */
   WriteConsoleA (get_output_handle (), wpbuf, wpixput, &wn, 0);
@@ -2989,6 +2995,36 @@ fhandler_console::write (const void *vsrc, size_t len)
       con.saw_space = false;
       con.saw_exclamation_mark = false;
     }
+  else if (*src == '8') /* DECRC Restore cursor position */
+    {
+      if (con.screen_alternated)
+ {
+  /* For xterm mode only */
+  DWORD n;
+  /* Just send the sequence */
+  wpbuf_put (*src);
+  WriteConsoleA (get_output_handle (), wpbuf, wpixput, &n, 0);
+ }
+      else if (con.savex >= 0 && con.savey >= 0)
+ cursor_set (false, con.savex, con.savey);
+      con.state = normal;
+      wpixput = 0;
+    }
+  else if (*src == '7') /* DECSC Save cursor position */
+    {
+      if (con.screen_alternated)
+ {
+  /* For xterm mode only */
+  DWORD n;
+  /* Just send the sequence */
+  wpbuf_put (*src);
+  WriteConsoleA (get_output_handle (), wpbuf, wpixput, &n, 0);
+ }
+      else
+ cursor_get (&con.savex, &con.savey);
+      con.state = normal;
+      wpixput = 0;
+    }
   else if (wincap.has_con_24bit_colors () && !con_is_legacy
    && wincap.has_con_broken_il_dl () && *src == 'M')
     { /* Reverse Index (scroll down) */
@@ -3019,12 +3055,15 @@ fhandler_console::write (const void *vsrc, size_t len)
       wpixput = 0;
     }
   else if (wincap.has_con_24bit_colors () && !con_is_legacy)
-    { /* Only CSI is handled in xterm compatible mode. */
+    {
       if (*src == 'c') /* RIS Full reset */
  {
   con.scroll_region.Top = 0;
   con.scroll_region.Bottom = -1;
  }
+      /* ESC sequences below (e.g. OSC, etc) are left to xterm
+ emulation in xterm compatible mode, therefore, are not
+ handled and just sent them. */
       wpbuf_put (*src);
       /* Just send the sequence */
       DWORD n;
@@ -3067,18 +3106,6 @@ fhandler_console::write (const void *vsrc, size_t len)
       con.state = normal;
       wpixput = 0;
     }
-  else if (*src == '8') /* DECRC Restore cursor position */
-    {
-      cursor_set (false, con.savex, con.savey);
-      con.state = normal;
-      wpixput = 0;
-    }
-  else if (*src == '7') /* DECSC Save cursor position */
-    {
-      cursor_get (&con.savex, &con.savey);
-      con.state = normal;
-      wpixput = 0;
-    }
   else if (*src == 'R') /* ? */
     {
       con.state = normal;
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/4] Cygwin: console: Some fixes for console in xterm mode.

Corinna Vinschen-2
In reply to this post by Takashi Yano
On Mar  2 10:12, Takashi Yano wrote:

> Takashi Yano (4):
>   Cygwin: console: Revise the code to fix tab position.
>   Cygwin: console: Fix setting/unsetting xterm mode for input.
>   Cygwin: console: Prevent buffer overrun.
>   Cygwin: console: Add a workaround for "ESC 7" and "ESC 8".
>
>  winsup/cygwin/fhandler.h          |  1 +
>  winsup/cygwin/fhandler_console.cc | 91 ++++++++++++++++++-------------
>  2 files changed, 55 insertions(+), 37 deletions(-)
>
> --
> 2.21.0
Pushed.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment