[PATCH v2 0/1] Cygwin: console: Revive Win7 compatibility.

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

[PATCH v2 0/1] Cygwin: console: Revive Win7 compatibility.

Takashi Yano
- The commit fca4cda7a420d7b15ac217d008527e029d05758e broke Win7
  compatibility. This patch fixes the issue.

v2:
Move definition of INREC_SIZE into fhandler.h from fhandler_console.cc
and select.cc.

Takashi Yano (1):
  Cygwin: console: Revive Win7 compatibility.

 winsup/cygwin/fhandler.h          | 6 ++++++
 winsup/cygwin/fhandler_console.cc | 6 ------
 winsup/cygwin/select.cc           | 1 -
 3 files changed, 6 insertions(+), 7 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 1/1] Cygwin: console: Revive Win7 compatibility.

Takashi Yano
- The commit fca4cda7a420d7b15ac217d008527e029d05758e broke Win7
  compatibility. This patch fixes the issue.
---
 winsup/cygwin/fhandler.h          | 6 ++++++
 winsup/cygwin/fhandler_console.cc | 6 ------
 winsup/cygwin/select.cc           | 1 -
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 4efb6a4f2..94b0e520b 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -43,6 +43,12 @@ details. */
 
 #define O_TMPFILE_FILE_ATTRS (FILE_ATTRIBUTE_TEMPORARY | FILE_ATTRIBUTE_HIDDEN)
 
+/* Buffer size for ReadConsoleInput() and PeakConsoleInput(). */
+/* Per MSDN, max size of buffer required is below 64K. */
+/* (65536 / sizeof (INPUT_RECORD)) is 3276, however,
+   ERROR_NOT_ENOUGH_MEMORY occurs in win7 if this value is used. */
+#define INREC_SIZE 2048
+
 extern const char *windows_device_names[];
 extern struct __cygwin_perfile *perfile_table;
 #define __fmode (*(user_data->fmode_ptr))
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 709b8255d..86c39db25 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -499,9 +499,6 @@ fhandler_console::process_input_message (void)
 
   termios *ti = &(get_ttyp ()->ti);
 
-  /* Per MSDN, max size of buffer required is below 64K. */
-#define  INREC_SIZE (65536 / sizeof (INPUT_RECORD))
-
   fhandler_console::input_states stat = input_processing;
   DWORD total_read, i;
   INPUT_RECORD input_rec[INREC_SIZE];
@@ -1165,9 +1162,6 @@ fhandler_console::ioctl (unsigned int cmd, void *arg)
  return -1;
       case FIONREAD:
  {
-  /* Per MSDN, max size of buffer required is below 64K. */
-#define  INREC_SIZE (65536 / sizeof (INPUT_RECORD))
-
   DWORD n;
   int ret = 0;
   INPUT_RECORD inp[INREC_SIZE];
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index ed8c98d1c..e7014422b 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1209,7 +1209,6 @@ peek_pty_slave (select_record *s, bool from_select)
  {
   if (ptys->is_line_input ())
     {
-#define INREC_SIZE (65536 / sizeof (INPUT_RECORD))
       INPUT_RECORD inp[INREC_SIZE];
       DWORD n;
       PeekConsoleInput (ptys->get_handle (), inp, INREC_SIZE, &n);
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/1] Cygwin: console: Revive Win7 compatibility.

Yaakov Selkowitz-2
On Thu, 2019-09-19 at 05:49 +0900, Takashi Yano wrote:

> - The commit fca4cda7a420d7b15ac217d008527e029d05758e broke Win7
>   compatibility. This patch fixes the issue.
> ---
>  winsup/cygwin/fhandler.h          | 6 ++++++
>  winsup/cygwin/fhandler_console.cc | 6 ------
>  winsup/cygwin/select.cc           | 1 -
>  3 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
> index 4efb6a4f2..94b0e520b 100644
> --- a/winsup/cygwin/fhandler.h
> +++ b/winsup/cygwin/fhandler.h
> @@ -43,6 +43,12 @@ details. */
>  
>  #define O_TMPFILE_FILE_ATTRS (FILE_ATTRIBUTE_TEMPORARY | FILE_ATTRIBUTE_HIDDEN)
>  
> +/* Buffer size for ReadConsoleInput() and PeakConsoleInput(). */
> +/* Per MSDN, max size of buffer required is below 64K. */
> +/* (65536 / sizeof (INPUT_RECORD)) is 3276, however,
> +   ERROR_NOT_ENOUGH_MEMORY occurs in win7 if this value is used. */
> +#define INREC_SIZE 2048
> +

Would it make sense to define this using wincap so it is 2048 for
Win7/2K8 and 3276 for newer versions?

>  extern const char *windows_device_names[];
>  extern struct __cygwin_perfile *perfile_table;
>  #define __fmode (*(user_data->fmode_ptr))
> diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
> index 709b8255d..86c39db25 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -499,9 +499,6 @@ fhandler_console::process_input_message (void)
>  
>    termios *ti = &(get_ttyp ()->ti);
>  
> -  /* Per MSDN, max size of buffer required is below 64K. */
> -#define  INREC_SIZE (65536 / sizeof (INPUT_RECORD))
> -
>    fhandler_console::input_states stat = input_processing;
>    DWORD total_read, i;
>    INPUT_RECORD input_rec[INREC_SIZE];
> @@ -1165,9 +1162,6 @@ fhandler_console::ioctl (unsigned int cmd, void *arg)
>   return -1;
>        case FIONREAD:
>   {
> -  /* Per MSDN, max size of buffer required is below 64K. */
> -#define  INREC_SIZE (65536 / sizeof (INPUT_RECORD))
> -
>    DWORD n;
>    int ret = 0;
>    INPUT_RECORD inp[INREC_SIZE];
> diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
> index ed8c98d1c..e7014422b 100644
> --- a/winsup/cygwin/select.cc
> +++ b/winsup/cygwin/select.cc
> @@ -1209,7 +1209,6 @@ peek_pty_slave (select_record *s, bool from_select)
>   {
>    if (ptys->is_line_input ())
>      {
> -#define INREC_SIZE (65536 / sizeof (INPUT_RECORD))
>        INPUT_RECORD inp[INREC_SIZE];
>        DWORD n;
>        PeekConsoleInput (ptys->get_handle (), inp, INREC_SIZE, &n);

--
Yaakov Selkowitz
Senior Software Engineer - Platform Enablement
Red Hat, Inc.


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/1] Cygwin: console: Revive Win7 compatibility.

Takashi Yano
On Wed, 18 Sep 2019 17:31:30 -0400
Yaakov Selkowitz wrote:
> Would it make sense to define this using wincap so it is 2048 for
> Win7/2K8 and 3276 for newer versions?

Thanks for advice. Of cource it is possible, however, I don't think
it is necessary. IMHO, the buffer size of 2048 is more than enough
for console keyboard input. Even if the number of records exceeds
2048, call of process_input_message() will be divided into several
transactions without loss of data. Previously, process_input_message()
was called byte by byte, so the current implementation is much better
than that even with the buffer whose depth is 2048.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/1] Cygwin: console: Revive Win7 compatibility.

Ken Brown-6
In reply to this post by Takashi Yano
On 9/18/2019 4:49 PM, Takashi Yano wrote:

> - The commit fca4cda7a420d7b15ac217d008527e029d05758e broke Win7
>    compatibility. This patch fixes the issue.
> ---
>   winsup/cygwin/fhandler.h          | 6 ++++++
>   winsup/cygwin/fhandler_console.cc | 6 ------
>   winsup/cygwin/select.cc           | 1 -
>   3 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
> index 4efb6a4f2..94b0e520b 100644
> --- a/winsup/cygwin/fhandler.h
> +++ b/winsup/cygwin/fhandler.h
> @@ -43,6 +43,12 @@ details. */
>  
>   #define O_TMPFILE_FILE_ATTRS (FILE_ATTRIBUTE_TEMPORARY | FILE_ATTRIBUTE_HIDDEN)
>  
> +/* Buffer size for ReadConsoleInput() and PeakConsoleInput(). */
                                               Peek

> +/* Per MSDN, max size of buffer required is below 64K. */
> +/* (65536 / sizeof (INPUT_RECORD)) is 3276, however,
> +   ERROR_NOT_ENOUGH_MEMORY occurs in win7 if this value is used. */
> +#define INREC_SIZE 2048
> +
>   extern const char *windows_device_names[];
>   extern struct __cygwin_perfile *perfile_table;
>   #define __fmode (*(user_data->fmode_ptr))
> diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
> index 709b8255d..86c39db25 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -499,9 +499,6 @@ fhandler_console::process_input_message (void)
>  
>     termios *ti = &(get_ttyp ()->ti);
>  
> -  /* Per MSDN, max size of buffer required is below 64K. */
> -#define  INREC_SIZE (65536 / sizeof (INPUT_RECORD))
> -
>     fhandler_console::input_states stat = input_processing;
>     DWORD total_read, i;
>     INPUT_RECORD input_rec[INREC_SIZE];
> @@ -1165,9 +1162,6 @@ fhandler_console::ioctl (unsigned int cmd, void *arg)
>   return -1;
>         case FIONREAD:
>   {
> -  /* Per MSDN, max size of buffer required is below 64K. */
> -#define  INREC_SIZE (65536 / sizeof (INPUT_RECORD))
> -
>    DWORD n;
>    int ret = 0;
>    INPUT_RECORD inp[INREC_SIZE];
> diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
> index ed8c98d1c..e7014422b 100644
> --- a/winsup/cygwin/select.cc
> +++ b/winsup/cygwin/select.cc
> @@ -1209,7 +1209,6 @@ peek_pty_slave (select_record *s, bool from_select)
>   {
>    if (ptys->is_line_input ())
>      {
> -#define INREC_SIZE (65536 / sizeof (INPUT_RECORD))
>        INPUT_RECORD inp[INREC_SIZE];
>        DWORD n;
>        PeekConsoleInput (ptys->get_handle (), inp, INREC_SIZE, &n);

Pushed, with the above typo corrected.  Thanks.

Ken