Re: [PATCH v2 1/1] Cygwin: console: Revive Win7 compatibility.
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. */ > -#defineINREC_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. */ > -#defineINREC_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
Re: [PATCH v2 1/1] Cygwin: console: Revive Win7 compatibility.
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
Re: [PATCH v2 1/1] Cygwin: console: Revive Win7 compatibility.
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. */ > -#defineINREC_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. */ > -#defineINREC_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.