[binutils-gdb/gdb-12-branch] gdb: move setbuf calls out of gdb_readline_no_editing_callback

[binutils-gdb/gdb-12-branch] gdb: move setbuf calls out of gdb_readline_no_editing_callback

[Joel Brobecker]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e312dfbe9556d8e4557abe7fd3f1a4681e15da06

commit e312dfbe9556d8e4557abe7fd3f1a4681e15da06
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sun Apr 24 08:39:19 2022 -0700

    gdb: move setbuf calls out of gdb_readline_no_editing_callback
    
    After this commit:
    
      commit d08cbc5d3203118da5583296e49273cf82378042
      Date:   Wed Dec 22 12:57:44 2021 +0000
    
          gdb: unbuffer all input streams when not using readline
    
    Issues were reported with some MS-Windows hosts, see the thread
    starting here:
    
      https://sourceware.org/pipermail/gdb-patches/2022-March/187004.html
    
    Filed in bugzilla as: PR mi/29002
    
    The problem seems to be that calling setbuf on terminal file handles
    is not always acceptable, see this mail for more details:
    
      https://sourceware.org/pipermail/gdb-patches/2022-April/187310.html
    
    This commit does two things, first moving the setbuf calls out of
    gdb_readline_no_editing_callback so that we don't end up calling
    setbuf so often.
    
    Then, for MS-Windows hosts, we don't call setbuf for terminals, this
    appears to resolve the issues that have been reported.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29002

Diff:
---
 gdb/event-top.c | 13 -------------
 gdb/top.c       | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index c1a95a4b748..e924d10fa02 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -821,19 +821,6 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
   FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
   gdb_assert (stream != nullptr);
 
-  /* Unbuffer the input stream, so that, later on, the calls to fgetc
-     fetch only one char at the time from the stream.  The fgetc's will
-     get up to the first newline, but there may be more chars in the
-     stream after '\n'.  If we buffer the input and fgetc drains the
-     stream, getting stuff beyond the newline as well, a select, done
-     afterwards will not trigger.
-
-     This unbuffering was, at one point, not applied if the input stream
-     was a tty, however, the buffering can cause problems, even for a tty,
-     in some cases.  Please ensure that any changes in this area run the MI
-     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
-  setbuf (stream, NULL);
-
   /* We still need the while loop here, even though it would seem
      obvious to invoke gdb_readline_no_editing_callback at every
      character entered.  If not using the readline library, the
diff --git a/gdb/top.c b/gdb/top.c
index 7831b4f96ca..3b39b7d1bfe 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -260,6 +260,41 @@ void (*deprecated_context_hook) (int id);
 /* The highest UI number ever assigned.  */
 static int highest_ui_num;
 
+/* Unbuffer STREAM.  This is a wrapper around setbuf(STREAM, nullptr)
+   which applies some special rules for MS-Windows hosts.  */
+
+static void
+unbuffer_stream (FILE *stream)
+{
+  /* Unbuffer the input stream so that in gdb_readline_no_editing_callback,
+     the calls to fgetc fetch only one char at the time from STREAM.
+
+     This is important because gdb_readline_no_editing_callback will read
+     from STREAM up to the first '\n' character, after this GDB returns to
+     the event loop and relies on a select on STREAM indicating that more
+     input is pending.
+
+     If STREAM is buffered then the fgetc calls may have moved all the
+     pending input from the kernel into a local buffer, after which the
+     select will not indicate that more input is pending, and input after
+     the first '\n' will not be processed immediately.
+
+     Please ensure that any changes in this area run the MI tests with the
+     FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
+
+#ifdef __MINGW32__
+  /* With MS-Windows runtime, making stdin unbuffered when it's
+     connected to the terminal causes it to misbehave.  */
+  if (!ISATTY (stream))
+    setbuf (stream, nullptr);
+#else
+  /* On GNU/Linux the issues described above can impact GDB even when
+     dealing with input from a terminal.  For now we unbuffer the input
+     stream for everyone except MS-Windows.  */
+  setbuf (stream, nullptr);
+#endif
+}
+
 /* See top.h.  */
 
 ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
@@ -286,6 +321,8 @@ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
 {
   buffer_init (&line_buffer);
 
+  unbuffer_stream (instream_);
+
   if (ui_list == NULL)
     ui_list = this;
   else
@@ -415,6 +452,8 @@ read_command_file (FILE *stream)
 {
   struct ui *ui = current_ui;
 
+  unbuffer_stream (stream);
+
   scoped_restore save_instream
     = make_scoped_restore (&ui->instream, stream);