public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Unbuffer stdout and stderr on windows
@ 2013-07-22  3:07 Yao Qi
  2013-07-22 15:41 ` Eli Zaretskii
  0 siblings, 1 reply; 44+ messages in thread
From: Yao Qi @ 2013-07-22  3:07 UTC (permalink / raw)
  To: gdb-patches

Hi,
This patch is to disable the buffering on windows host, because the
error message and gdb prompt come out in different orders, which
causes a lot of test fails.

We call setvbuf this place, because it is a place "before any other
operation is performed".  See the doc below:

  "The setvbuf() function may be used after the stream pointed to by
stream is associated with an open file but before any other operation
(other than an unsuccessful call to setvbuf()) is performed on the
stream."

It is not the first time this patch show up here.  Daniel posted it
http://sourceware.org/ml/gdb-patches/2009-06/msg00433.html and Joel
preferred it as the exact same piece of code is in their tree as well
http://sourceware.org/ml/gdb-patches/2009-06/msg00434.html
Eli wanted to check this patch didn't interfere with Emacs 23 GDB
interface on Windows, which is probably the last question to this
patch.  The discussion stopped there.  I build native mingw32 gdb
with buffering disabled, and use it with Emacs 24.3 in Windows
cmd.exe.  Emacs+GDB behave correctly.

Is it OK?

gdb:

2013-07-22  Joseph Myers  <joseph@codesourcery.com>

	* main.c (captured_main): Set stdout and stderr unbuffered on
	Windows.
---
 gdb/main.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 440094e..7529dc5 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -375,6 +375,13 @@ captured_main (void *data)
   saved_command_line[0] = '\0';
   instream = stdin;
 
+#ifdef _WIN32
+  /* A Cygwin ssh session may not look like a terminal to the Windows
+     runtime; ensure unbuffered output.  */
+  setvbuf (stdout, NULL, _IONBF, BUFSIZ);
+  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+#endif
+
   gdb_stdout = stdio_fileopen (stdout);
   gdb_stderr = stdio_fileopen (stderr);
   gdb_stdlog = gdb_stderr;	/* for moment */
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-22  3:07 [PATCH] Unbuffer stdout and stderr on windows Yao Qi
@ 2013-07-22 15:41 ` Eli Zaretskii
  2013-07-23  6:35   ` Yao Qi
  0 siblings, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-07-22 15:41 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

> From: Yao Qi <yao@codesourcery.com>
> Date: Mon, 22 Jul 2013 11:06:57 +0800
> 
> +#ifdef _WIN32
> +  /* A Cygwin ssh session may not look like a terminal to the Windows
> +     runtime; ensure unbuffered output.  */
> +  setvbuf (stdout, NULL, _IONBF, BUFSIZ);
> +  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
> +#endif

This is wrong, stdout should be line-buffered by default.

Also, this punishes batch mode with stdout redirected to a file: its
stdout buffering (and perhaps also that of stderr, although that's
less important) will now always be line-buffered, i.e. less efficient.

Is it possible to detect the "Cygwin ssh session", whatever that
means, and only do this then?  I don't think it's right to change
behavior of a native w32 GDB just because it misbehaves when mixed
with Cygwin.  Mixing native and Cygwin programs is asking for trouble
to begin with, so punishing good citizens on behalf of that corner
case is not TRT, IMO.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-22 15:41 ` Eli Zaretskii
@ 2013-07-23  6:35   ` Yao Qi
  2013-07-23 17:52     ` Eli Zaretskii
  0 siblings, 1 reply; 44+ messages in thread
From: Yao Qi @ 2013-07-23  6:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 07/22/2013 11:40 PM, Eli Zaretskii wrote:
> This is wrong, stdout should be line-buffered by default.
>

stdout is line-buffered if it can be detected to connect to an 
interactive device, otherwise it is fully buffered.  Cygwin emulates pty 
with pipes, and native win32 apps blindly assume all pipes are 
non-interactive.

> Also, this punishes batch mode with stdout redirected to a file: its
> stdout buffering (and perhaps also that of stderr, although that's
> less important) will now always be line-buffered, i.e. less efficient.

Yeah, this patch hurts the performance, but gets the outputs in the 
correct order, so that testsuite can be run to get a reasonable test result.

>
> Is it possible to detect the "Cygwin ssh session", whatever that
> means, and only do this then?  I don't think it's right to change

Unfortunately, I am unable to find a heuristics to tell "GDB is in 
cygwin session".  There are some differences on env variables between 
cygwin and Windows cmd.exe console, but I am afraid that they are not 
reliable.

> behavior of a native w32 GDB just because it misbehaves when mixed

This patch is to change the buffered output to unbuffered, so the 
behaviour of GDB is not changed, IMO.

> with Cygwin.  Mixing native and Cygwin programs is asking for trouble
> to begin with, so punishing good citizens on behalf of that corner
> case is not TRT, IMO.

We test mingw native gdb in cygwin, because it is easy to set up, so 
that mingw32 native gdb can be tested more widely.  In the last resort, 
we may add an option "--cygwin-tty", which I don't really like.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-23  6:35   ` Yao Qi
@ 2013-07-23 17:52     ` Eli Zaretskii
  2013-07-29 19:26       ` Christopher Faylor
  0 siblings, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-07-23 17:52 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

> Date: Tue, 23 Jul 2013 14:34:32 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 07/22/2013 11:40 PM, Eli Zaretskii wrote:
> > This is wrong, stdout should be line-buffered by default.
> >
> 
> stdout is line-buffered if it can be detected to connect to an 
> interactive device, otherwise it is fully buffered.  Cygwin emulates pty 
> with pipes, and native win32 apps blindly assume all pipes are 
> non-interactive.

Sorry, I was unclear: I meant that stdout should be line-buffered, not
unbuffered, when it is connected to a console (which is the default).
I was under the impression that you were changing stdout to be
unbuffered even in that default case, i.e. even when stdout is
connected to a console.  If that's not so, i.e. if you were changing
the buffering only when stdout is not connected to a console, I
apologize for my misunderstanding.

> > Also, this punishes batch mode with stdout redirected to a file: its
> > stdout buffering (and perhaps also that of stderr, although that's
> > less important) will now always be line-buffered, i.e. less efficient.
> 
> Yeah, this patch hurts the performance, but gets the outputs in the 
> correct order, so that testsuite can be run to get a reasonable test result.

I don't think it's right to penalize users for the benefit of the test
suite.

> > Is it possible to detect the "Cygwin ssh session", whatever that
> > means, and only do this then?  I don't think it's right to change
> 
> Unfortunately, I am unable to find a heuristics to tell "GDB is in 
> cygwin session".  There are some differences on env variables between 
> cygwin and Windows cmd.exe console, but I am afraid that they are not 
> reliable.

Then how about introducing a special option which will cause the
buffering to be what you want?  The test suite could use that option,
and the other users will not suffer any penalty.

> > behavior of a native w32 GDB just because it misbehaves when mixed
> 
> This patch is to change the buffered output to unbuffered, so the 
> behaviour of GDB is not changed, IMO.

Of course it's changed: buffering is user-visible behavior.

> In the last resort, we may add an option "--cygwin-tty"

I think this is the best alternative.

> which I don't really like.

Why not?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-23 17:52     ` Eli Zaretskii
@ 2013-07-29 19:26       ` Christopher Faylor
  2013-07-29 19:30         ` Eli Zaretskii
  2013-07-29 19:30         ` Christopher Faylor
  0 siblings, 2 replies; 44+ messages in thread
From: Christopher Faylor @ 2013-07-29 19:26 UTC (permalink / raw)
  To: gdb-patches, Yao Qi, Eli Zaretskii

On Tue, Jul 23, 2013 at 08:51:53PM +0300, Eli Zaretskii wrote:
>> Date: Tue, 23 Jul 2013 14:34:32 +0800
>> From: Yao Qi <yao@codesourcery.com>
>> CC: <gdb-patches@sourceware.org>
>> 
>> On 07/22/2013 11:40 PM, Eli Zaretskii wrote:
>> > This is wrong, stdout should be line-buffered by default.
>> >
>> 
>> stdout is line-buffered if it can be detected to connect to an 
>> interactive device, otherwise it is fully buffered.  Cygwin emulates pty 
>> with pipes, and native win32 apps blindly assume all pipes are 
>> non-interactive.
>
>Sorry, I was unclear: I meant that stdout should be line-buffered, not
>unbuffered, when it is connected to a console (which is the default).
>I was under the impression that you were changing stdout to be
>unbuffered even in that default case, i.e. even when stdout is
>connected to a console.  If that's not so, i.e. if you were changing
>the buffering only when stdout is not connected to a console, I
>apologize for my misunderstanding.
>
>> > Also, this punishes batch mode with stdout redirected to a file: its
>> > stdout buffering (and perhaps also that of stderr, although that's
>> > less important) will now always be line-buffered, i.e. less efficient.
>> 
>> Yeah, this patch hurts the performance, but gets the outputs in the 
>> correct order, so that testsuite can be run to get a reasonable test result.
>
>I don't think it's right to penalize users for the benefit of the test
>suite.
>
>> > Is it possible to detect the "Cygwin ssh session", whatever that
>> > means, and only do this then?  I don't think it's right to change
>> 
>> Unfortunately, I am unable to find a heuristics to tell "GDB is in 
>> cygwin session".  There are some differences on env variables between 
>> cygwin and Windows cmd.exe console, but I am afraid that they are not 
>> reliable.
>
>Then how about introducing a special option which will cause the
>buffering to be what you want?  The test suite could use that option,
>and the other users will not suffer any penalty.
>
>> > behavior of a native w32 GDB just because it misbehaves when mixed
>> 
>> This patch is to change the buffered output to unbuffered, so the 
>> behaviour of GDB is not changed, IMO.
>
>Of course it's changed: buffering is user-visible behavior.
>
>> In the last resort, we may add an option "--cygwin-tty"
>
>I think this is the best alternative.
>
>> which I don't really like.
>
>Why not?

Note that this is only a problem for non-Cygwin versions of gdb.  It
sounds like someone is using mingw-built versions of gdb in a cygwin
pty session.  This obviously wouldn't be something that Cygwin itself
would need.

How about a "--buffer-output" option instead, which defaults to true?

cgf

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-29 19:26       ` Christopher Faylor
  2013-07-29 19:30         ` Eli Zaretskii
@ 2013-07-29 19:30         ` Christopher Faylor
  1 sibling, 0 replies; 44+ messages in thread
From: Christopher Faylor @ 2013-07-29 19:30 UTC (permalink / raw)
  To: gdb-patches, Yao Qi, Eli Zaretskii

On Mon, Jul 29, 2013 at 03:25:59PM -0400, Christopher Faylor wrote:
>...

Nevermind.  Should have read the rest of the messages about this.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-29 19:26       ` Christopher Faylor
@ 2013-07-29 19:30         ` Eli Zaretskii
  2013-07-29 19:51           ` Pedro Alves
  2013-07-29 19:30         ` Christopher Faylor
  1 sibling, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-07-29 19:30 UTC (permalink / raw)
  To: gdb-patches, yao

> Date: Mon, 29 Jul 2013 15:25:59 -0400
> From: Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org>
> 
> Note that this is only a problem for non-Cygwin versions of gdb.  It
> sounds like someone is using mingw-built versions of gdb in a cygwin
> pty session.

Yes.  Specifically, an attempt to run a MinGW GDB through the test
suite using Cygwin expect.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-29 19:30         ` Eli Zaretskii
@ 2013-07-29 19:51           ` Pedro Alves
  2013-07-31  3:40             ` Christopher Faylor
  0 siblings, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-07-29 19:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, yao

On 07/29/2013 08:30 PM, Eli Zaretskii wrote:
>> Date: Mon, 29 Jul 2013 15:25:59 -0400
>> From: Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org>
>>
>> Note that this is only a problem for non-Cygwin versions of gdb.  It
>> sounds like someone is using mingw-built versions of gdb in a cygwin
>> pty session.
> 
> Yes.  Specifically, an attempt to run a MinGW GDB through the test
> suite using Cygwin expect.

IMO, the ultimate solution is to run the testsuite with a small
netcat-like wrapper program that creates a console, redirects
the child's stdin/stdout/stderr to the console, and bridges data
between the cygwin pipes and the console.

 cygwin (io=tty/pipe) <-> [ pipe <-> console ] <-> gdb (io=console)

The testsuite would then spawn "wrapper.exe gdb.exe ..." instead of
"gdb.exe".

GDB would then see a regular console for stdin/stdout/stderr, and
work out of the box.  PR15791 points at winpty, an open source
tool that from the description does that (and more).  It'd be nice
if that path would be investigated.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-29 19:51           ` Pedro Alves
@ 2013-07-31  3:40             ` Christopher Faylor
  2013-08-12 21:11               ` Joel Brobecker
  0 siblings, 1 reply; 44+ messages in thread
From: Christopher Faylor @ 2013-07-31  3:40 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches, yao, Eli Zaretskii

On Mon, Jul 29, 2013 at 08:51:14PM +0100, Pedro Alves wrote:
>On 07/29/2013 08:30 PM, Eli Zaretskii wrote:
>>> Date: Mon, 29 Jul 2013 15:25:59 -0400
>>> From: Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org>
>>>
>>> Note that this is only a problem for non-Cygwin versions of gdb.  It
>>> sounds like someone is using mingw-built versions of gdb in a cygwin
>>> pty session.
>> 
>> Yes.  Specifically, an attempt to run a MinGW GDB through the test
>> suite using Cygwin expect.
>
>IMO, the ultimate solution is to run the testsuite with a small
>netcat-like wrapper program that creates a console, redirects
>the child's stdin/stdout/stderr to the console, and bridges data
>between the cygwin pipes and the console.
>
> cygwin (io=tty/pipe) <-> [ pipe <-> console ] <-> gdb (io=console)
>
>The testsuite would then spawn "wrapper.exe gdb.exe ..." instead of
>"gdb.exe".
>
>GDB would then see a regular console for stdin/stdout/stderr, and
>work out of the box.  PR15791 points at winpty, an open source
>tool that from the description does that (and more).  It'd be nice
>if that path would be investigated.

We had a somewhat heated debate in the cygwin list about using the
techniques in winpty and eventually abandoned the idea because the way
things like winpty create consoles is not foolproof.  Since it relies on
polling, it is theoretically possible to lose data.

I'll bet that, in practice you'd never see any data loss, though.
And, from that observation, you can see which side of the argument
I was on.  :-)

cgf

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-07-31  3:40             ` Christopher Faylor
@ 2013-08-12 21:11               ` Joel Brobecker
  2013-08-13 17:28                 ` Christopher Faylor
  2013-08-13 18:08                 ` Eli Zaretskii
  0 siblings, 2 replies; 44+ messages in thread
From: Joel Brobecker @ 2013-08-12 21:11 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches, yao, Eli Zaretskii

> We had a somewhat heated debate in the cygwin list about using the
> techniques in winpty and eventually abandoned the idea because the way
> things like winpty create consoles is not foolproof.  Since it relies on
> polling, it is theoretically possible to lose data.
> 
> I'll bet that, in practice you'd never see any data loss, though.
> And, from that observation, you can see which side of the argument
> I was on.  :-)

FWIW, many frontends also implements communication with GDB using
pipes on Windows, and running MinGW-gdb inside cygwin window/shell
is just a very very common practice, regardless of whether officially
supported or not. How does Emacs do, for instance? IIRC when I looked
at the code, that's what it did.

Having the stdout/stderr output mixed up is very confusing and breaks
testing as well, so we applied the same approach as Yao's at AdaCore.
In the many many years that we've used this approach, no one ever
complained to us about standard I/O performance.

For completeness, our calls to setvbuf are inserted about 10 lines
later, after gdb_stdout/gdb_stderr are set up.

-- 
Joel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-12 21:11               ` Joel Brobecker
@ 2013-08-13 17:28                 ` Christopher Faylor
  2013-08-13 18:08                 ` Eli Zaretskii
  1 sibling, 0 replies; 44+ messages in thread
From: Christopher Faylor @ 2013-08-13 17:28 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches, yao, Joel Brobecker, Eli Zaretskii

On Mon, Aug 12, 2013 at 02:11:05PM -0700, Joel Brobecker wrote:
>> We had a somewhat heated debate in the cygwin list about using the
>> techniques in winpty and eventually abandoned the idea because the way
>> things like winpty create consoles is not foolproof.  Since it relies on
>> polling, it is theoretically possible to lose data.
>> 
>> I'll bet that, in practice you'd never see any data loss, though.
>> And, from that observation, you can see which side of the argument
>> I was on.  :-)
>
>FWIW, many frontends also implements communication with GDB using
>pipes on Windows, and running MinGW-gdb inside cygwin window/shell
>is just a very very common practice, regardless of whether officially
>supported or not. How does Emacs do, for instance? IIRC when I looked
>at the code, that's what it did.
>
>Having the stdout/stderr output mixed up is very confusing and breaks
>testing as well, so we applied the same approach as Yao's at AdaCore.
>In the many many years that we've used this approach, no one ever
>complained to us about standard I/O performance.
>
>For completeness, our calls to setvbuf are inserted about 10 lines
>later, after gdb_stdout/gdb_stderr are set up.

So do you just unconditionally set gdb_std* to unbuffered?  I would
think that this would be nearly unnoticeable.

cgf

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-12 21:11               ` Joel Brobecker
  2013-08-13 17:28                 ` Christopher Faylor
@ 2013-08-13 18:08                 ` Eli Zaretskii
  2013-08-14  0:05                   ` Joel Brobecker
  2013-08-15 17:36                   ` Christopher Faylor
  1 sibling, 2 replies; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-13 18:08 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: palves, gdb-patches, yao

> Date: Mon, 12 Aug 2013 14:11:05 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> > We had a somewhat heated debate in the cygwin list about using the
> > techniques in winpty and eventually abandoned the idea because the way
> > things like winpty create consoles is not foolproof.  Since it relies on
> > polling, it is theoretically possible to lose data.
> > 
> > I'll bet that, in practice you'd never see any data loss, though.
> > And, from that observation, you can see which side of the argument
> > I was on.  :-)
> 
> FWIW, many frontends also implements communication with GDB using
> pipes on Windows, and running MinGW-gdb inside cygwin window/shell
> is just a very very common practice, regardless of whether officially
> supported or not. How does Emacs do, for instance? IIRC when I looked
> at the code, that's what it did.

Yes, Emacs does that.  But it is never a problem in that case, because
it's the user who looks at the results, not a program that wants to
interpret them.

> Having the stdout/stderr output mixed up is very confusing and breaks
> testing as well, so we applied the same approach as Yao's at AdaCore.

Making GDB output unbuffered is not a good idea for Emacs, because it
will cause it read single characters, which is (a) inefficient, and
(b) error-prone, because a single CR character could dupe the text
decoding routines into thinking the EOL format is from Mac.

I think this problem is only relevant to the test suite, where a
program tries to interpret the output and trips on extraneous
characters.  Let's not make the solution so much wider than the
problem.  With Yao's patch, you now have this in a Cygwin terminal;
can we leave all the other types alone, please?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-13 18:08                 ` Eli Zaretskii
@ 2013-08-14  0:05                   ` Joel Brobecker
  2013-08-15 17:36                   ` Christopher Faylor
  1 sibling, 0 replies; 44+ messages in thread
From: Joel Brobecker @ 2013-08-14  0:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, gdb-patches, yao

> Yes, Emacs does that.  But it is never a problem in that case, because
> it's the user who looks at the results, not a program that wants to
> interpret them.
[...]
> I think this problem is only relevant to the test suite,

Actually, I did find that having stdout and stderr mixed up to
be very confusing. But I know I've never found a solution that
would be acceptable to everyone. This is why I've never formally
submitted the patch, only published it as an FYI the few times
someone asked about the issue. Credits to Dan Jacobowitz, btw,
for giving it to me in the first place.

> program tries to interpret the output and trips on extraneous
> characters.  Let's not make the solution so much wider than the
> problem.  With Yao's patch, you now have this in a Cygwin terminal;
> can we leave all the other types alone, please?

Sure. I don't see how this is going to help the testsuite, but
Yao's patch is already progress, and we've been using a vendor patch
for years anyway, so we can continue doing so without problems.

-- 
Joel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-13 18:08                 ` Eli Zaretskii
  2013-08-14  0:05                   ` Joel Brobecker
@ 2013-08-15 17:36                   ` Christopher Faylor
  2013-08-15 17:44                     ` Eli Zaretskii
  1 sibling, 1 reply; 44+ messages in thread
From: Christopher Faylor @ 2013-08-15 17:36 UTC (permalink / raw)
  To: palves, gdb-patches, yao, Joel Brobecker, Eli Zaretskii

On Tue, Aug 13, 2013 at 09:08:38PM +0300, Eli Zaretskii wrote:
>> Date: Mon, 12 Aug 2013 14:11:05 -0700
>> From: Joel Brobecker <brobecker@adacore.com>
>> 
>> > We had a somewhat heated debate in the cygwin list about using the
>> > techniques in winpty and eventually abandoned the idea because the way
>> > things like winpty create consoles is not foolproof.  Since it relies on
>> > polling, it is theoretically possible to lose data.
>> > 
>> > I'll bet that, in practice you'd never see any data loss, though.
>> > And, from that observation, you can see which side of the argument
>> > I was on.  :-)
>> 
>> FWIW, many frontends also implements communication with GDB using
>> pipes on Windows, and running MinGW-gdb inside cygwin window/shell
>> is just a very very common practice, regardless of whether officially
>> supported or not. How does Emacs do, for instance? IIRC when I looked
>> at the code, that's what it did.
>
>Yes, Emacs does that.  But it is never a problem in that case, because
>it's the user who looks at the results, not a program that wants to
>interpret them.
>
>> Having the stdout/stderr output mixed up is very confusing and breaks
>> testing as well, so we applied the same approach as Yao's at AdaCore.
>
>Making GDB output unbuffered is not a good idea for Emacs, because it
>will cause it read single characters, which is (a) inefficient, and
>(b) error-prone, because a single CR character could dupe the text
>decoding routines into thinking the EOL format is from Mac.

I thought that "unbuffered" normally means something like "every output
operation gets immediately sent as a block" rather than "flush
after every character".  If the mingw "unbuffered" mode means that
everything is o n e  c h a r a c t e r  a t  a  t i m e, then that
would obviously not be an acceptable solution.  Or, maybe this is just
the way emacs itself works.

The other alternative would be to use line buffering for gdb.  I don't
see why cygwin pipes (whether they are "ptys" or actual pipes) are a
special case here.  stdout is usually line buffered isn't it?  Why not
just force that behavior for gdb?

cgf

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-15 17:36                   ` Christopher Faylor
@ 2013-08-15 17:44                     ` Eli Zaretskii
  2013-08-15 17:59                       ` Christopher Faylor
  0 siblings, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-15 17:44 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: palves, gdb-patches, yao, brobecker

> Date: Thu, 15 Aug 2013 13:36:18 -0400
> From: Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org>
> 
> I thought that "unbuffered" normally means something like "every output
> operation gets immediately sent as a block" rather than "flush
> after every character".

AFAIK, unbuffered always meant the latter.

> If the mingw "unbuffered" mode means that everything is o n e c h a
> r a c t e r a t a t i m e

It does mean that.  Doesn't it work like that in Cygwin?

> maybe this is just the way emacs itself works.

Emacs sits in a pselect call waiting for input, when input is
available, it reads it.  If input is available one character at a
time, it will be read in very small chunks.

> The other alternative would be to use line buffering for gdb.  I don't
> see why cygwin pipes (whether they are "ptys" or actual pipes) are a
> special case here.  stdout is usually line buffered isn't it?  Why not
> just force that behavior for gdb?

That's what I suggested, but Yao says that using line buffering still
fails some tests.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-15 17:44                     ` Eli Zaretskii
@ 2013-08-15 17:59                       ` Christopher Faylor
  2013-08-15 18:44                         ` Eli Zaretskii
  2013-08-16 11:46                         ` Pedro Alves
  0 siblings, 2 replies; 44+ messages in thread
From: Christopher Faylor @ 2013-08-15 17:59 UTC (permalink / raw)
  To: palves, gdb-patches, brobecker, yao, Eli Zaretskii

On Thu, Aug 15, 2013 at 08:44:43PM +0300, Eli Zaretskii wrote:
>On Thu, 15 Aug 2013 13:36:18 -0400, Christopher Faylor wrote:
>>I thought that "unbuffered" normally means something like "every output
>>operation gets immediately sent as a block" rather than "flush after
>>every character".
>
>AFAIK, unbuffered always meant the latter.
>
>>If the mingw "unbuffered" mode means that everything is o n e c h a r a
>>c t e r a t a t i m e
>
>It does mean that.  Doesn't it work like that in Cygwin?

Cygwin uses newlib which, AFAICT, writes a block at a time without
storing the block in a buffer first.

So:

   fwrite (foo, 27, 1, stdout);

writes 27 bytes to stdout in one shot, without buffering.

I only got this from looking at the code so I could be wrong.

>>The other alternative would be to use line buffering for gdb.  I don't
>>see why cygwin pipes (whether they are "ptys" or actual pipes) are a
>>special case here.  stdout is usually line buffered isn't it?  Why not
>>just force that behavior for gdb?
>
>That's what I suggested, but Yao says that using line buffering still
>fails some tests.

Sorry, I missed that you'd already suggested that.

cgf

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-15 17:59                       ` Christopher Faylor
@ 2013-08-15 18:44                         ` Eli Zaretskii
  2013-08-16 11:46                         ` Pedro Alves
  1 sibling, 0 replies; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-15 18:44 UTC (permalink / raw)
  To: palves, gdb-patches, brobecker, yao

> Date: Thu, 15 Aug 2013 13:59:40 -0400
> From: Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org>
> 
> >>If the mingw "unbuffered" mode means that everything is o n e c h a r a
> >>c t e r a t a t i m e
> >
> >It does mean that.  Doesn't it work like that in Cygwin?
> 
> Cygwin uses newlib which, AFAICT, writes a block at a time without
> storing the block in a buffer first.
> 
> So:
> 
>    fwrite (foo, 27, 1, stdout);
> 
> writes 27 bytes to stdout in one shot, without buffering.

Sorry, I wasn't thinking about fwrite.  I was thinking about fprintf
and putc.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-15 17:59                       ` Christopher Faylor
  2013-08-15 18:44                         ` Eli Zaretskii
@ 2013-08-16 11:46                         ` Pedro Alves
  2013-08-16 12:34                           ` Yao Qi
  2013-08-16 13:17                           ` Eli Zaretskii
  1 sibling, 2 replies; 44+ messages in thread
From: Pedro Alves @ 2013-08-16 11:46 UTC (permalink / raw)
  To: palves, gdb-patches, brobecker, yao, Eli Zaretskii

Sorry for chiming in so late...

On 08/15/2013 06:59 PM, Christopher Faylor wrote:
> On Thu, Aug 15, 2013 at 08:44:43PM +0300, Eli Zaretskii wrote:
>> On Thu, 15 Aug 2013 13:36:18 -0400, Christopher Faylor wrote:
>>> I thought that "unbuffered" normally means something like "every output
>>> operation gets immediately sent as a block" rather than "flush after
>>> every character".
>>
>> AFAIK, unbuffered always meant the latter.
>>
>>> If the mingw "unbuffered" mode means that everything is o n e c h a r a
>>> c t e r a t a t i m e
>>
>> It does mean that.  Doesn't it work like that in Cygwin?
> 
> Cygwin uses newlib which, AFAICT, writes a block at a time without
> storing the block in a buffer first.
> 
> So:
> 
>    fwrite (foo, 27, 1, stdout);
> 
> writes 27 bytes to stdout in one shot, without buffering.
> 
> I only got this from looking at the code so I could be wrong.
> 
>>> The other alternative would be to use line buffering for gdb.  I don't
>>> see why cygwin pipes (whether they are "ptys" or actual pipes) are a
>>> special case here.  stdout is usually line buffered isn't it?  Why not
>>> just force that behavior for gdb?
>>
>> That's what I suggested, but Yao says that using line buffering still
>> fails some tests.
> 
> Sorry, I missed that you'd already suggested that.

Quoting that part of previous discussion:

On 08/01/2013 09:05 AM, Yao Qi wrote:
> On 07/29/2013 11:42 PM, Eli Zaretskii wrote:

>>>> +      setvbuf (stdout, NULL, _IONBF, BUFSIZ);
>>>> +      setvbuf (stderr, NULL, _IONBF, BUFSIZ);
>> How about using line buffering instead on both streams?  Or at least
>> leave stdout line-buffered?  Did you try that, and if so, did that
>> have the same problems that triggered these patches?
>
> I tried line-buffering for stdout, but get some regressions in python
> related testing,

That's because there's no such thing as line-buffering on Windows.
See <http://msdn.microsoft.com/en-us/library/86cebhfs%28v=vs.71%29.aspx>

  _IOLBF
      For some systems, this provides line buffering. However, for Win32, the behavior
      is the same as _IOFBF - Full Buffering.


Also, ISO C (C99/TC3/N1256, 7.19.3 Files, point 7) says:

  "As initially opened, the standard error stream is not fully buffered;
  the standard input and standard output streams are fully buffered if
  and only if the stream can be determined not to refer to an interactive
  device."

Note that stderr might be either unbuffered or line buffered,
but _never_ fully buffered.  And most implementations default to leaving
stderr always unbuffered (so that error output comes out immediately).

However, the Windows runtime, in its infinite wisdom, makes stderr
fully buffered if connected to a pipe.

So all this makes me very much question the desire to detect
if a native Win32 GDB is running under Cygwin.

IMO, stderr should _always_ be forced to unbuffered.

I can't really imagine that leaving stdout fully buffered to
ever be good (which the cygwin detection seems to want to preserve),
even for frontends, given GDB is an interactive program, and even
MI is text/line based.

So I think the "in cygwin" detection is really not necessary
or desirable, and this patch should go back to its original form:

+#ifdef _WIN32
+  /* A Cygwin ssh session may not look like a terminal to the Windows
+     runtime; ensure unbuffered output.  */
+  setvbuf (stdout, NULL, _IONBF, BUFSIZ);
+  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+#endif

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 11:46                         ` Pedro Alves
@ 2013-08-16 12:34                           ` Yao Qi
  2013-08-16 13:20                             ` Eli Zaretskii
  2013-08-16 13:37                             ` Pedro Alves
  2013-08-16 13:17                           ` Eli Zaretskii
  1 sibling, 2 replies; 44+ messages in thread
From: Yao Qi @ 2013-08-16 12:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, brobecker, Eli Zaretskii

On 08/16/2013 07:46 PM, Pedro Alves wrote:
> However, the Windows runtime, in its infinite wisdom, makes stderr
> fully buffered if connected to a pipe.
>
> So all this makes me very much question the desire to detect
> if a native Win32 GDB is running under Cygwin.
>
> IMO, stderr should_always_  be forced to unbuffered.

 From this long discussion, people agree that stderr should be unbuffered.

>
> I can't really imagine that leaving stdout fully buffered to
> ever be good (which the cygwin detection seems to want to preserve),
> even for frontends, given GDB is an interactive program, and even
> MI is text/line based.
>
> So I think the "in cygwin" detection is really not necessary
> or desirable, and this patch should go back to its original form:

However, we didn't have an agreement on what to set for stdout.  As you 
posted above, on Win32, stdout can be either full buffered or 
non-buffered.  If we change stdout to non-buffered, there are two 
concerns, 1) GDB will slow down, 2) cause troubles to front-end, like Emacs.

Is it useful to reduce the concerns by measuring the slow down and 
testing patched GDB under Emacs?

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 11:46                         ` Pedro Alves
  2013-08-16 12:34                           ` Yao Qi
@ 2013-08-16 13:17                           ` Eli Zaretskii
  2013-08-16 13:30                             ` Pedro Alves
  1 sibling, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 13:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, brobecker, yao

> Date: Fri, 16 Aug 2013 12:46:17 +0100
> From: Pedro Alves <palves@redhat.com>
> 
> IMO, stderr should _always_ be forced to unbuffered.

We are in agreement about stderr.  The issue is with stdout.

> I can't really imagine that leaving stdout fully buffered to
> ever be good (which the cygwin detection seems to want to preserve),
> even for frontends, given GDB is an interactive program, and even
> MI is text/line based.

If the choice is between fully buffered stdout and unbuffered stdout,
I prefer the former for performance reasons.

> So I think the "in cygwin" detection is really not necessary
> or desirable, and this patch should go back to its original form:
> 
> +#ifdef _WIN32
> +  /* A Cygwin ssh session may not look like a terminal to the Windows
> +     runtime; ensure unbuffered output.  */
> +  setvbuf (stdout, NULL, _IONBF, BUFSIZ);
> +  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
> +#endif

Sorry, I disagree.  No one has ever complained about the current
buffering of stdout on Windows.  The issue that is the subject of this
thread is a problem that happens in running the test suite.  I firmly
believe in the principle of not fixing that which isn't broken.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 12:34                           ` Yao Qi
@ 2013-08-16 13:20                             ` Eli Zaretskii
  2013-08-16 13:37                             ` Pedro Alves
  1 sibling, 0 replies; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 13:20 UTC (permalink / raw)
  To: Yao Qi; +Cc: palves, gdb-patches, brobecker

> Date: Fri, 16 Aug 2013 20:33:56 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>, <brobecker@adacore.com>, Eli Zaretskii
> 	<eliz@gnu.org>
> 
> However, we didn't have an agreement on what to set for stdout.  As you 
> posted above, on Win32, stdout can be either full buffered or 
> non-buffered.  If we change stdout to non-buffered, there are two 
> concerns, 1) GDB will slow down, 2) cause troubles to front-end, like Emacs.
> 
> Is it useful to reduce the concerns by measuring the slow down and 
> testing patched GDB under Emacs?

Not sure such a research will be worth our while.

How about a command-line option to do this, something that was
proposed before?  Would people feel less divided about that?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 13:17                           ` Eli Zaretskii
@ 2013-08-16 13:30                             ` Pedro Alves
  2013-08-16 13:42                               ` Eli Zaretskii
  0 siblings, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-08-16 13:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, brobecker, yao

On 08/16/2013 02:17 PM, Eli Zaretskii wrote:
>> Date: Fri, 16 Aug 2013 12:46:17 +0100
>> From: Pedro Alves <palves@redhat.com>
>>
>> IMO, stderr should _always_ be forced to unbuffered.
> 
> We are in agreement about stderr.  The issue is with stdout.

OK.

>> I can't really imagine that leaving stdout fully buffered to
>> ever be good (which the cygwin detection seems to want to preserve),
>> even for frontends, given GDB is an interactive program, and even
>> MI is text/line based.
> 
> If the choice is between fully buffered stdout and unbuffered stdout,
> I prefer the former for performance reasons.
> 
>> So I think the "in cygwin" detection is really not necessary
>> or desirable, and this patch should go back to its original form:

...

> Sorry, I disagree.  No one has ever complained about the current
> buffering of stdout on Windows.  The issue that is the subject of this
> thread is a problem that happens in running the test suite.  I firmly
> believe in the principle of not fixing that which isn't broken.

So how is emacs starting GDB?  Because if it is starting GDB
in a console, then nobody's complaining because output is already
not buffered.  If it is started in a pipe, then what is
forcing flushing at line ends so that emacs doesn't break?
IOW, the testsuite is not special, in that if emacs starts
GDB with io connected to pipes, then it should be having the
exact same issues the testsuite has.  In that case, hacking
the issue _only_ when running the testsuite actually masks
out a real problem.  We do have "gdb_flush (gdb_stdout)" calls
sprinkled through the tree.  If that is what makes emacs+pipe work,
then OK, we can remain with stdout fully buffered, but then I'd say
that the bug is that there are places that miss gdb_flush calls.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 12:34                           ` Yao Qi
  2013-08-16 13:20                             ` Eli Zaretskii
@ 2013-08-16 13:37                             ` Pedro Alves
  2013-08-16 14:03                               ` Eli Zaretskii
  1 sibling, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-08-16 13:37 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, brobecker, Eli Zaretskii

On 08/16/2013 01:33 PM, Yao Qi wrote:
> On 08/16/2013 07:46 PM, Pedro Alves wrote:
>> However, the Windows runtime, in its infinite wisdom, makes stderr
>> fully buffered if connected to a pipe.
>>
>> So all this makes me very much question the desire to detect
>> if a native Win32 GDB is running under Cygwin.
>>
>> IMO, stderr should_always_  be forced to unbuffered.
> 
>  From this long discussion, people agree that stderr should be unbuffered.
> 
>>
>> I can't really imagine that leaving stdout fully buffered to
>> ever be good (which the cygwin detection seems to want to preserve),
>> even for frontends, given GDB is an interactive program, and even
>> MI is text/line based.
>>
>> So I think the "in cygwin" detection is really not necessary
>> or desirable, and this patch should go back to its original form:
> 
> However, we didn't have an agreement on what to set for stdout.  As you 
> posted above, on Win32, stdout can be either full buffered or 
> non-buffered.  If we change stdout to non-buffered, there are two 
> concerns, 1) GDB will slow down, 2) cause troubles to front-end, like Emacs.

How can it cause trouble?  If anything, it should be fully buffered
that causes trouble (for output not coming out when it should).  I
really can't see when ever would it be desirable to hold GDB output
until the internal buffer fills.  So if GDB is supposed to be working when
IO is fully buffered, some other mechanism must be forcing flushes on
line ends (probably the gdb_flush calls).  And then, if that is supposed
to work, the testsuite really isn't special, and what we have is that
the flushing mechanism isn't being activated in all the necessary places.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 13:30                             ` Pedro Alves
@ 2013-08-16 13:42                               ` Eli Zaretskii
  2013-08-16 14:13                                 ` Pedro Alves
  0 siblings, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 13:42 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, brobecker, yao

> Date: Fri, 16 Aug 2013 14:30:20 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org, brobecker@adacore.com, yao@codesourcery.com
> 
> So how is emacs starting GDB?

Via a pipe, of course.  How else could Emacs read what GDB outputs and
interpret it?

> If it is started in a pipe, then what is forcing flushing at line
> ends so that emacs doesn't break?

Emacs has a special machinery to decode EOLs correctly even if input
ends in a CR, and the NL comes later.  But this machinery is
heuristics and it's fragile.  Right now, the amount of such "broken"
outputs is usually very small, and the fact that GDB flushes its
stdout probably makes it negligibly small.  If we cause more of these
instances to happen, the probability of a failure in a given session
will go up.

> IOW, the testsuite is not special, in that if emacs starts
> GDB with io connected to pipes, then it should be having the
> exact same issues the testsuite has.

The test suite is special, AFAIU.  The original problem was not with
buffering, it was with mixing stdout and stderr that confused Expect.
An Emacs (human) user will never be confused by any such mix, and I
believe MI output makes the mix improbable.

In any case, I heard no complaints about any such situations, nor did
I ever see that myself.

> We do have "gdb_flush (gdb_stdout)" calls sprinkled through the
> tree.  If that is what makes emacs+pipe work, then OK, we can remain
> with stdout fully buffered, but then I'd say that the bug is that
> there are places that miss gdb_flush calls.

Maybe we should dig deeper in the original problem as well, because I
still have only a very vague notion of why would GDB, which is a
single-threaded program, cause mixing when it flushes stdout
regularly.  What am I missing?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 13:37                             ` Pedro Alves
@ 2013-08-16 14:03                               ` Eli Zaretskii
  2013-08-16 14:21                                 ` Pedro Alves
  0 siblings, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 14:03 UTC (permalink / raw)
  To: Pedro Alves; +Cc: yao, gdb-patches, brobecker

> Date: Fri, 16 Aug 2013 14:37:23 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org, brobecker@adacore.com,
>         Eli Zaretskii <eliz@gnu.org>
> 
> How can it cause trouble?  If anything, it should be fully buffered
> that causes trouble (for output not coming out when it should).  I
> really can't see when ever would it be desirable to hold GDB output
> until the internal buffer fills.  So if GDB is supposed to be working when
> IO is fully buffered, some other mechanism must be forcing flushes on
> line ends (probably the gdb_flush calls).  And then, if that is supposed
> to work, the testsuite really isn't special, and what we have is that
> the flushing mechanism isn't being activated in all the necessary places.

If you are thinking about using GDB interactively from a Windows
console, these issues are not really applicable, because console
output has its own rules.  And, as you point out, GDB flushes its
stdout quite a lot anyway.  So the current stdout should really behave
close to line-buffered.

I suggest not to broaden the issue beyond the original problem, lest
we will be unable to solve it.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 13:42                               ` Eli Zaretskii
@ 2013-08-16 14:13                                 ` Pedro Alves
  2013-08-16 14:44                                   ` Eli Zaretskii
  0 siblings, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-08-16 14:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, brobecker, yao

On 08/16/2013 02:42 PM, Eli Zaretskii wrote:
>> Date: Fri, 16 Aug 2013 14:30:20 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org, brobecker@adacore.com, yao@codesourcery.com
>>
>> So how is emacs starting GDB?
> 
> Via a pipe, of course.  How else could Emacs read what GDB outputs and
> interpret it?

Dunno, perhaps some hidden console magic.

>> If it is started in a pipe, then what is forcing flushing at line
>> ends so that emacs doesn't break?
> 
> Emacs has a special machinery to decode EOLs correctly even if input
> ends in a CR, and the NL comes later.  But this machinery is
> heuristics and it's fragile.  

That sounds like the opposite scenario.  That is, GDB sending "\r\n",
and emacs getting "\r" first, putting it in an internal line buffer,
and then waiting for "\n" for the line to be complete.  That's doing
based line based input reading when the lower level routines don't do
it for you.  I don't see how to get around this if you have either
non-buffered or fully buffered output as in both cases, you have
reads returning data around characters that are not line endings.

> Right now, the amount of such "broken"
> outputs is usually very small, and the fact that GDB flushes its
> stdout probably makes it negligibly small.  If we cause more of these
> instances to happen, the probability of a failure in a given session
> will go up.

Not sure what you mean by causing more instances to happen?

> 
>> IOW, the testsuite is not special, in that if emacs starts
>> GDB with io connected to pipes, then it should be having the
>> exact same issues the testsuite has.
> 
> The test suite is special, AFAIU.  The original problem was not with
> buffering, it was with mixing stdout and stderr that confused Expect.
> An Emacs (human) user will never be confused by any such mix, and I
> believe MI output makes the mix improbable.

It seems to me the mixing is a direct consequence of buffering.
With buffering of stdout and stderr, the runtime will push chunks
of lines to both stdout and stderr when the internal buffers are full,
with no regard to line endings.  IOW, if gdb does:

 "line1\nline2\n" -> stdout
 "LINE3\nLINE4\n" -> stderr

then depending on when is the internal stdout and stderr
buffers filled, the runtime may well flush it like:

"line1\nliNE3\nLINE4\nn", and then "e2\n"

If it's not this, but whole lines being swapped somehow,
maybe always flushing stdout before outputting stderr
and vice versa is likely to fix it, I'd guess?  Even if the swapping
is completely whole-line based somehow, I still think that the testsuite
is not special.  Just because emacs doesn't _care_ that the output
might be swapped, it's still a fact that it is, and the testsuite
exercises what consumers of GDB output see.

> In any case, I heard no complaints about any such situations, nor did
> I ever see that myself.
> 
>> We do have "gdb_flush (gdb_stdout)" calls sprinkled through the
>> tree.  If that is what makes emacs+pipe work, then OK, we can remain
>> with stdout fully buffered, but then I'd say that the bug is that
>> there are places that miss gdb_flush calls.
> 
> Maybe we should dig deeper in the original problem as well, because I
> still have only a very vague notion of why would GDB, which is a
> single-threaded program, cause mixing when it flushes stdout
> regularly.  What am I missing?

I think so too.  A paste of the mixed output would be good
to start.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 14:03                               ` Eli Zaretskii
@ 2013-08-16 14:21                                 ` Pedro Alves
  2013-08-16 14:57                                   ` Eli Zaretskii
  0 siblings, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-08-16 14:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yao, gdb-patches, brobecker

On 08/16/2013 03:03 PM, Eli Zaretskii wrote:

> I suggest not to broaden the issue beyond the original problem, lest
> we will be unable to solve it.

Nobody's doing that.  What I'm saying is that the testsuite is not
special, and we should _zone in_ to the real problem, and fix it
completely.  Making output not buffered seems to do that, assuming
GDB doesn't output half a line to stdout, then output to stderr, and
then output the rest to stderr, which very much it will want to avoid,
given stderr is unbuffered on most systems.  If the output is buffered, then
something must be forcing flushes to get the same effect.  Fixing
the issue for the testsuite but not for other GDB-consumers that
spawn it in a pipe, with special modes is just not the right
solution, IMO.  What will probably happen with that solution is
that Windows GDB clients will end up using the special testsuite-only
flag too, because by default, GDB will remain broken...  Why add
a flag for a half-fix instead of fixing the issue completely?

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 14:13                                 ` Pedro Alves
@ 2013-08-16 14:44                                   ` Eli Zaretskii
  2013-08-16 15:05                                     ` Pedro Alves
  0 siblings, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 14:44 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, brobecker, yao

> Date: Fri, 16 Aug 2013 15:13:49 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org, brobecker@adacore.com, yao@codesourcery.com
> 
> > Right now, the amount of such "broken"
> > outputs is usually very small, and the fact that GDB flushes its
> > stdout probably makes it negligibly small.  If we cause more of these
> > instances to happen, the probability of a failure in a given session
> > will go up.
> 
> Not sure what you mean by causing more instances to happen?

I mean by that having more smaller chunks of text come out of GDB's
end of the pipe.

> It seems to me the mixing is a direct consequence of buffering.
> With buffering of stdout and stderr, the runtime will push chunks
> of lines to both stdout and stderr when the internal buffers are full,
> with no regard to line endings.  IOW, if gdb does:
> 
>  "line1\nline2\n" -> stdout
>  "LINE3\nLINE4\n" -> stderr
> 
> then depending on when is the internal stdout and stderr
> buffers filled, the runtime may well flush it like:
> 
> "line1\nliNE3\nLINE4\nn", and then "e2\n"

Yes, but since we flush stdout, the above shouldn't happen.

> > Maybe we should dig deeper in the original problem as well, because I
> > still have only a very vague notion of why would GDB, which is a
> > single-threaded program, cause mixing when it flushes stdout
> > regularly.  What am I missing?
> 
> I think so too.  A paste of the mixed output would be good
> to start.

Agreed.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 14:21                                 ` Pedro Alves
@ 2013-08-16 14:57                                   ` Eli Zaretskii
  2013-08-16 15:10                                     ` Pedro Alves
  0 siblings, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 14:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: yao, gdb-patches, brobecker

> Date: Fri, 16 Aug 2013 15:21:50 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: yao@codesourcery.com, gdb-patches@sourceware.org, brobecker@adacore.com
> 
> On 08/16/2013 03:03 PM, Eli Zaretskii wrote:
> 
> > I suggest not to broaden the issue beyond the original problem, lest
> > we will be unable to solve it.
> 
> Nobody's doing that.  What I'm saying is that the testsuite is not
> special, and we should _zone in_ to the real problem, and fix it
> completely.

A complete solution, if it exists, might require much more effort,
which is IMO unjustified in the absence of complaints.

> What will probably happen with that solution is that Windows GDB
> clients will end up using the special testsuite-only flag too,
> because by default, GDB will remain broken...

I use the Emacs client all the time, and I don't think it's broken.


> Why add a flag for a half-fix instead of fixing the issue
> completely?

I fear that we don't know/understand enough to do TRT for the complete
solution.  The "half-fix", OTOH, solves a well-defined issue that is
understood very well.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 14:44                                   ` Eli Zaretskii
@ 2013-08-16 15:05                                     ` Pedro Alves
  2013-08-16 15:13                                       ` Eli Zaretskii
  0 siblings, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-08-16 15:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, brobecker, yao

On 08/16/2013 03:44 PM, Eli Zaretskii wrote:
>> Date: Fri, 16 Aug 2013 15:13:49 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org, brobecker@adacore.com, yao@codesourcery.com
>>
>>> Right now, the amount of such "broken"
>>> outputs is usually very small, and the fact that GDB flushes its
>>> stdout probably makes it negligibly small.  If we cause more of these
>>> instances to happen, the probability of a failure in a given session
>>> will go up.
>>
>> Not sure what you mean by causing more instances to happen?
> 
> I mean by that having more smaller chunks of text come out of GDB's
> end of the pipe.

Ack.  I can't say I really agree with this rationale.  The problem
exists, and hiding it under the rug won't make it go away.  Actually
forcing it to happen more frequently should make it easier to
iron out all the kinks.  IIUC, this is just a layer that
splits input into whole lines before passing it up to higher
levels, thus emulating line buffering.  I'm failing to see why
that is heuristic, rather than something that works 100% of the
time, given that that sounds just like fgets.

> 
>> It seems to me the mixing is a direct consequence of buffering.
>> With buffering of stdout and stderr, the runtime will push chunks
>> of lines to both stdout and stderr when the internal buffers are full,
>> with no regard to line endings.  IOW, if gdb does:
>>
>>  "line1\nline2\n" -> stdout
>>  "LINE3\nLINE4\n" -> stderr
>>
>> then depending on when is the internal stdout and stderr
>> buffers filled, the runtime may well flush it like:
>>
>> "line1\nliNE3\nLINE4\nn", and then "e2\n"
> 
> Yes, but since we flush stdout, the above shouldn't happen.

Clearly something like that must be happening, otherwise
the testsuite wouldn't have problems.  It just sounds like we
don't actually flush stdout _or_ stderr in all the places
we need to, although the existence of many flushes in the code
show that that's the intent of the existing solution to this problem.

I found this email from Joel at:

http://sourceware.org/ml/gdb-patches/2009-06/msg00447.html

Saying:

 "... the piece I quoted only
 unbuffers stdout and stderr, so that output sent on both file handles
 do not get printed out of order (in other words, if we print on
 stderr first, and then stdout, we want the output to be in that
 order - with buffering, we observed that stderr output was printed
 after stdout output, even if the actual call to printf was in a
 different order)."

So that actually supports my theory, and, my earlier suggestion
that if fully unbuffered is undesirable, then instead make sure to
always flush stdout before switching output to stderr, and vice
versa, so the order is preserved.  That should fix the issue, and,
avoid the potential (though I'm not sure how real) drop of
performance with going fully unbuffered, and, potentially fixes
the issue for everyone, testsuite and frontends included.
The question, IMO, if it the potential performance hit of going
fully unbuffered is worth it, but I still think that if yes,
the performance issue is real, then the fix should be a generic
one, rather then a testsuite hack.  I don't imagine such a fix to
be a big patch.  It could e.g., go somewhere around fputs_unfiltered
or some such.

>>> Maybe we should dig deeper in the original problem as well, because I
>>> still have only a very vague notion of why would GDB, which is a
>>> single-threaded program, cause mixing when it flushes stdout
>>> regularly.  What am I missing?
>>
>> I think so too.  A paste of the mixed output would be good
>> to start.
> 
> Agreed.
> 

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 14:57                                   ` Eli Zaretskii
@ 2013-08-16 15:10                                     ` Pedro Alves
  2013-08-16 15:24                                       ` Pedro Alves
                                                         ` (2 more replies)
  0 siblings, 3 replies; 44+ messages in thread
From: Pedro Alves @ 2013-08-16 15:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yao, gdb-patches, brobecker

On 08/16/2013 03:57 PM, Eli Zaretskii wrote:
>> Date: Fri, 16 Aug 2013 15:21:50 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: yao@codesourcery.com, gdb-patches@sourceware.org, brobecker@adacore.com
>>
>> On 08/16/2013 03:03 PM, Eli Zaretskii wrote:
>>
>>> I suggest not to broaden the issue beyond the original problem, lest
>>> we will be unable to solve it.
>>
>> Nobody's doing that.  What I'm saying is that the testsuite is not
>> special, and we should _zone in_ to the real problem, and fix it
>> completely.
> 
> A complete solution, if it exists, might require much more effort,

I don't think it does.

- First, make stderr always unbuffered.  That's what you get on
most platforms anyway.
- Then, somewhere along fputs_unfiltered or some central output routine,
keep track of which was the last to be used between stdout and stderr.
If outputting to stderr, flush stdout first.  Don't need to do the
opposite, since stderr will always be unbuffered.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 15:05                                     ` Pedro Alves
@ 2013-08-16 15:13                                       ` Eli Zaretskii
  0 siblings, 0 replies; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 15:13 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, brobecker, yao

> Date: Fri, 16 Aug 2013 16:04:57 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org, brobecker@adacore.com, yao@codesourcery.com
> 
> Ack.  I can't say I really agree with this rationale.  The problem
> exists, and hiding it under the rug won't make it go away.  Actually
> forcing it to happen more frequently should make it easier to
> iron out all the kinks.  IIUC, this is just a layer that
> splits input into whole lines before passing it up to higher
> levels, thus emulating line buffering.  I'm failing to see why
> that is heuristic, rather than something that works 100% of the
> time, given that that sounds just like fgets.

Because Emacs decodes text as soon as it receives it, to show it to
users.  It could delay a bit, but not too much, because users will
complain.

> So that actually supports my theory, and, my earlier suggestion
> that if fully unbuffered is undesirable, then instead make sure to
> always flush stdout before switching output to stderr, and vice
> versa, so the order is preserved.

I think this is a good solution, if indeed it solves the original
problem.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 15:10                                     ` Pedro Alves
@ 2013-08-16 15:24                                       ` Pedro Alves
  2013-08-16 15:43                                         ` Eli Zaretskii
  2013-08-16 15:41                                       ` Eli Zaretskii
  2013-08-22  6:14                                       ` Yao Qi
  2 siblings, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-08-16 15:24 UTC (permalink / raw)
  To: gdb-patches; +Cc: Eli Zaretskii, yao, brobecker

On 08/16/2013 04:10 PM, Pedro Alves wrote:

>> A complete solution, if it exists, might require much more effort,
> 
> I don't think it does.
> 
> - First, make stderr always unbuffered.  That's what you get on
> most platforms anyway.
> - Then, somewhere along fputs_unfiltered or some central output routine,
> keep track of which was the last to be used between stdout and stderr.
> If outputting to stderr, flush stdout first.  Don't need to do the
> opposite, since stderr will always be unbuffered.

Or alternatively, like Tromey suggested, make fputs_unfiltered
or some such implement line buffering internally (for Windows hosts),
either by just forcing flushes at endlines (partial emulation), or
with real local buffering.  That potentially gets us a behavior
on Windows/pipes even closer to other platforms.  (His suggestion
was kind of sidetracked by the \n -> \r\n bit, but that's actually
a separate issue).

(Neither solution looks that complicated to me.)

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 15:10                                     ` Pedro Alves
  2013-08-16 15:24                                       ` Pedro Alves
@ 2013-08-16 15:41                                       ` Eli Zaretskii
  2013-08-22  6:14                                       ` Yao Qi
  2 siblings, 0 replies; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 15:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: yao, gdb-patches, brobecker

> Date: Fri, 16 Aug 2013 16:10:05 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: yao@codesourcery.com, gdb-patches@sourceware.org, brobecker@adacore.com
> 
> - First, make stderr always unbuffered.  That's what you get on
> most platforms anyway.
> - Then, somewhere along fputs_unfiltered or some central output routine,
> keep track of which was the last to be used between stdout and stderr.
> If outputting to stderr, flush stdout first.  Don't need to do the
> opposite, since stderr will always be unbuffered.

I'm okay with that, FWIW.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 15:24                                       ` Pedro Alves
@ 2013-08-16 15:43                                         ` Eli Zaretskii
  2013-08-16 16:41                                           ` Christopher Faylor
  0 siblings, 1 reply; 44+ messages in thread
From: Eli Zaretskii @ 2013-08-16 15:43 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, yao, brobecker

> Date: Fri, 16 Aug 2013 16:23:58 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: Eli Zaretskii <eliz@gnu.org>, yao@codesourcery.com, brobecker@adacore.com
> 
> Or alternatively, like Tromey suggested, make fputs_unfiltered
> or some such implement line buffering internally (for Windows hosts),
> either by just forcing flushes at endlines (partial emulation), or
> with real local buffering.

I'm okay with that, too.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 15:43                                         ` Eli Zaretskii
@ 2013-08-16 16:41                                           ` Christopher Faylor
  0 siblings, 0 replies; 44+ messages in thread
From: Christopher Faylor @ 2013-08-16 16:41 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches, brobecker, yao, Eli Zaretskii

On Fri, Aug 16, 2013 at 06:44:09PM +0300, Eli Zaretskii wrote:
>> Date: Fri, 16 Aug 2013 16:23:58 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: Eli Zaretskii <eliz@gnu.org>, yao@codesourcery.com, brobecker@adacore.com
>> 
>> Or alternatively, like Tromey suggested, make fputs_unfiltered
>> or some such implement line buffering internally (for Windows hosts),
>> either by just forcing flushes at endlines (partial emulation), or
>> with real local buffering.
>
>I'm okay with that, too.

Me too.

cgf

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-16 15:10                                     ` Pedro Alves
  2013-08-16 15:24                                       ` Pedro Alves
  2013-08-16 15:41                                       ` Eli Zaretskii
@ 2013-08-22  6:14                                       ` Yao Qi
  2013-08-22 14:18                                         ` Joel Brobecker
  2 siblings, 1 reply; 44+ messages in thread
From: Yao Qi @ 2013-08-22  6:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches, brobecker

On 08/16/2013 11:10 PM, Pedro Alves wrote:
> - First, make stderr always unbuffered.  That's what you get on
> most platforms anyway.
> - Then, somewhere along fputs_unfiltered or some central output routine,
> keep track of which was the last to be used between stdout and stderr.
> If outputting to stderr, flush stdout first.  Don't need to do the
> opposite, since stderr will always be unbuffered.

This patch below generally implements what we discussed here...

In this patch, we firstly set stderr unbuffered.  Then, we overwrite
gdb_stderr's methods 'to_write' and 'to_fputs', so that gdb_stdout
is flushed when something is written to gdb_stderr.

Regression tested on mingw32 native gdb with a local patch to set
stdout/stderr to binary mode.

-- 
Yao (齐尧)

gdb:

2013-08-22  Yao Qi  <yao@codesourcery.com>

	* main.c (captured_main) [__MINGW32__]: Set stderr unbuffered.
	and all update_stderr_ui_file.
	* ui-file.c (stderr_file_write): New function.
	(stderr_file_fputs): New function.
	(update_stderr_ui_file): New function.
	* ui-file.h (update_stderr_ui_file): Declare.
---
 gdb/main.c    |   19 +++++++++++++++++++
 gdb/ui-file.c |   26 ++++++++++++++++++++++++++
 gdb/ui-file.h |    3 +++
 3 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 1c240e4..8e00c27 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -375,8 +375,27 @@ captured_main (void *data)
   saved_command_line[0] = '\0';
   instream = stdin;
 
+#ifdef __MINGW32__
+  /* Ensure stderr is unbuffered.  */
+  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+#endif
+
   gdb_stdout = stdio_fileopen (stdout);
   gdb_stderr = stdio_fileopen (stderr);
+#ifdef __MINGW32__
+  /* There is no real line-buffering on Windows, see
+     http://msdn.microsoft.com/en-us/library/86cebhfs%28v=vs.71%29.aspx
+     so the stdout is either fully-buffered or non-buffered.  We can't
+     make stdout non-buffered, because of two concerns,
+     1.  non-buffering hurts performance,
+     2.  non-buffering may change GDB's behavior when it is interacting
+     with front-end, such as Emacs.
+
+     We decide to leave stdout as fully buffered, but flush it first
+     when something is written to stderr.  */
+  update_stderr_ui_file (gdb_stderr);
+#endif
+
   gdb_stdlog = gdb_stderr;	/* for moment */
   gdb_stdtarg = gdb_stderr;	/* for moment */
   gdb_stdin = stdio_fileopen (stdin);
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index cf5a86d..649d892 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -654,6 +654,32 @@ stdio_file_fseek (struct ui_file *file, long offset, int whence)
   return fseek (stdio->file, offset, whence);
 }
 
+static void
+stderr_file_write (struct ui_file *file, const char *buf, long length_buf)
+{
+  gdb_flush (gdb_stdout);
+  stdio_file_write (file, buf, length_buf);
+}
+
+static void
+stderr_file_fputs (const char *linebuffer, struct ui_file *file)
+{
+  gdb_flush (gdb_stdout);
+  stdio_file_fputs (linebuffer, file);
+}
+
+/* Overwrite UI_FILE's methods 'to_write' and 'to_fputs' by function
+   in which gdb_stdout is flushed.  */
+
+void
+update_stderr_ui_file (struct ui_file *ui_file)
+{
+  /* Method 'to_write_async_safe' is not overwritten, because it is
+     only used on linux host.  */
+  set_ui_file_write (ui_file, stderr_file_write);
+  set_ui_file_fputs (ui_file, stderr_file_fputs);
+}
+
 /* Like fdopen().  Create a ui_file from a previously opened FILE.  */
 
 struct ui_file *
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 9fef68c..6d96c0b 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -129,6 +129,9 @@ extern struct ui_file *mem_fileopen (void);
 /* Open/create a STDIO based UI_FILE using the already open FILE.  */
 extern struct ui_file *stdio_fileopen (FILE *file);
 
+extern void update_stderr_ui_file (struct ui_file *ui_file);
+
+
 /* Open NAME returning an STDIO based UI_FILE.  */
 extern struct ui_file *gdb_fopen (const char *name, const char *mode);
 
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-22  6:14                                       ` Yao Qi
@ 2013-08-22 14:18                                         ` Joel Brobecker
  2013-08-23  2:20                                           ` Yao Qi
  0 siblings, 1 reply; 44+ messages in thread
From: Joel Brobecker @ 2013-08-22 14:18 UTC (permalink / raw)
  To: Yao Qi; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches

> This patch below generally implements what we discussed here...

Thanks, Yao.

> 2013-08-22  Yao Qi  <yao@codesourcery.com>
> 
> 	* main.c (captured_main) [__MINGW32__]: Set stderr unbuffered.
> 	and all update_stderr_ui_file.
> 	* ui-file.c (stderr_file_write): New function.
> 	(stderr_file_fputs): New function.
> 	(update_stderr_ui_file): New function.
> 	* ui-file.h (update_stderr_ui_file): Declare.

Just some thoughts I had while skimming through your patch...

> +#ifdef __MINGW32__
> +  /* Ensure stderr is unbuffered.  */
> +  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
> +#endif

Can you also document the reason why you make stderr unbuffered here?
If we capture most of what was discussed, it'll help anyone trying
to understand the code later on...

>    gdb_stdout = stdio_fileopen (stdout);
>    gdb_stderr = stdio_fileopen (stderr);
> +#ifdef __MINGW32__
> +  /* There is no real line-buffering on Windows, see
> +     http://msdn.microsoft.com/en-us/library/86cebhfs%28v=vs.71%29.aspx
> +     so the stdout is either fully-buffered or non-buffered.  We can't
> +     make stdout non-buffered, because of two concerns,
> +     1.  non-buffering hurts performance,
> +     2.  non-buffering may change GDB's behavior when it is interacting
> +     with front-end, such as Emacs.
> +
> +     We decide to leave stdout as fully buffered, but flush it first
           decided
> +     when something is written to stderr.  */
> +  update_stderr_ui_file (gdb_stderr);
> +#endif

Just a question: Would it be possible to emulate line buffering
as well? In my experience, the size of the buffer is fairly big,
and causes the output to appear only at fairly large intervals,
and in fairly large blobs.


> +
>    gdb_stdlog = gdb_stderr;	/* for moment */
>    gdb_stdtarg = gdb_stderr;	/* for moment */
>    gdb_stdin = stdio_fileopen (stdin);
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index cf5a86d..649d892 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -654,6 +654,32 @@ stdio_file_fseek (struct ui_file *file, long offset, int whence)
>    return fseek (stdio->file, offset, whence);
>  }
>  
> +static void
> +stderr_file_write (struct ui_file *file, const char *buf, long length_buf)

Please, document every new function, no matter how trivial. That way,
there is no discussion about whether the function should be documented
or not.

Thank you,
-- 
Joel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-22 14:18                                         ` Joel Brobecker
@ 2013-08-23  2:20                                           ` Yao Qi
  2013-08-23 13:38                                             ` Joel Brobecker
  2013-08-27 20:39                                             ` Pedro Alves
  0 siblings, 2 replies; 44+ messages in thread
From: Yao Qi @ 2013-08-23  2:20 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches

Thanks for the review, Joel.

On 08/22/2013 10:17 PM, Joel Brobecker wrote:
>> +#ifdef __MINGW32__
>> >+  /* Ensure stderr is unbuffered.  */
>> >+  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
>> >+#endif
> Can you also document the reason why you make stderr unbuffered here?
> If we capture most of what was discussed, it'll help anyone trying
> to understand the code later on...
> 

Sure, comment is added.

>> >    gdb_stdout = stdio_fileopen (stdout);
>> >    gdb_stderr = stdio_fileopen (stderr);
>> >+#ifdef __MINGW32__
>> >+  /* There is no real line-buffering on Windows, see
>> >+http://msdn.microsoft.com/en-us/library/86cebhfs%28v=vs.71%29.aspx
>> >+     so the stdout is either fully-buffered or non-buffered.  We can't
>> >+     make stdout non-buffered, because of two concerns,
>> >+     1.  non-buffering hurts performance,
>> >+     2.  non-buffering may change GDB's behavior when it is interacting
>> >+     with front-end, such as Emacs.
>> >+
>> >+     We decide to leave stdout as fully buffered, but flush it first
>             decided

Fixed.

>> >+     when something is written to stderr.  */
>> >+  update_stderr_ui_file (gdb_stderr);
>> >+#endif
> Just a question: Would it be possible to emulate line buffering
> as well? In my experience, the size of the buffer is fairly big,
> and causes the output to appear only at fairly large intervals,
> and in fairly large blobs.
> 

I am not sure.  The goal of this patch is to keep the order of outputs
from stdout and stderr correct, so I don't think much on emulating
line buffering for stdout.  IMO, it can be addressed by a separated
patch.

> 
>> >+
>> >    gdb_stdlog = gdb_stderr;	/* for moment */
>> >    gdb_stdtarg = gdb_stderr;	/* for moment */
>> >    gdb_stdin = stdio_fileopen (stdin);
>> >diff --git a/gdb/ui-file.c b/gdb/ui-file.c
>> >index cf5a86d..649d892 100644
>> >--- a/gdb/ui-file.c
>> >+++ b/gdb/ui-file.c
>> >@@ -654,6 +654,32 @@ stdio_file_fseek (struct ui_file *file, long offset, int whence)
>> >    return fseek (stdio->file, offset, whence);
>> >  }
>> >  
>> >+static void
>> >+stderr_file_write (struct ui_file *file, const char *buf, long length_buf)
> Please, document every new function, no matter how trivial. That way,
> there is no discussion about whether the function should be documented
> or not.

OK, documented in the updated patch.

-- 
Yao (齐尧)

gdb:

2013-08-23  Yao Qi  <yao@codesourcery.com>

	* main.c (captured_main) [__MINGW32__]: Set stderr unbuffered.
	and all update_stderr_ui_file.
	* ui-file.c (stderr_file_write): New function.
	(stderr_file_fputs): New function.
	(update_stderr_ui_file): New function.
	* ui-file.h (update_stderr_ui_file): Declare.
---
 gdb/main.c    |   20 ++++++++++++++++++++
 gdb/ui-file.c |   32 ++++++++++++++++++++++++++++++++
 gdb/ui-file.h |    3 +++
 3 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 1c240e4..332df9e 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -375,8 +375,28 @@ captured_main (void *data)
   saved_command_line[0] = '\0';
   instream = stdin;
 
+#ifdef __MINGW32__
+  /* Ensure stderr is unbuffered.  A Cygwin pty or pipe is implemented
+     as a Windows pipe, and Windows buffers on pipes.  */
+  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+#endif
+
   gdb_stdout = stdio_fileopen (stdout);
   gdb_stderr = stdio_fileopen (stderr);
+#ifdef __MINGW32__
+  /* There is no real line-buffering on Windows, see
+     http://msdn.microsoft.com/en-us/library/86cebhfs%28v=vs.71%29.aspx
+     so the stdout is either fully-buffered or non-buffered.  We can't
+     make stdout non-buffered, because of two concerns,
+     1.  non-buffering hurts performance,
+     2.  non-buffering may change GDB's behavior when it is interacting
+     with front-end, such as Emacs.
+
+     We decided to leave stdout as fully buffered, but flush it first
+     when something is written to stderr.  */
+  update_stderr_ui_file (gdb_stderr);
+#endif
+
   gdb_stdlog = gdb_stderr;	/* for moment */
   gdb_stdtarg = gdb_stderr;	/* for moment */
   gdb_stdin = stdio_fileopen (stdin);
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index cf5a86d..4b26b78 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -654,6 +654,38 @@ stdio_file_fseek (struct ui_file *file, long offset, int whence)
   return fseek (stdio->file, offset, whence);
 }
 
+/* This is the implementation of ui_file method to_write for stderr.
+   gdb_stdout is flushed before writing to gdb_stderr.  */
+
+static void
+stderr_file_write (struct ui_file *file, const char *buf, long length_buf)
+{
+  gdb_flush (gdb_stdout);
+  stdio_file_write (file, buf, length_buf);
+}
+
+/* This is the implementation of ui_file method to_fputs for stderr.
+   gdb_stdout is flushed before writing to gdb_stderr.  */
+
+static void
+stderr_file_fputs (const char *linebuffer, struct ui_file *file)
+{
+  gdb_flush (gdb_stdout);
+  stdio_file_fputs (linebuffer, file);
+}
+
+/* Overwrite UI_FILE's methods 'to_write' and 'to_fputs' by function
+   in which gdb_stdout is flushed.  */
+
+void
+update_stderr_ui_file (struct ui_file *ui_file)
+{
+  /* Method 'to_write_async_safe' is not overwritten, because it is
+     only used on linux host.  */
+  set_ui_file_write (ui_file, stderr_file_write);
+  set_ui_file_fputs (ui_file, stderr_file_fputs);
+}
+
 /* Like fdopen().  Create a ui_file from a previously opened FILE.  */
 
 struct ui_file *
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 9fef68c..6d96c0b 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -129,6 +129,9 @@ extern struct ui_file *mem_fileopen (void);
 /* Open/create a STDIO based UI_FILE using the already open FILE.  */
 extern struct ui_file *stdio_fileopen (FILE *file);
 
+extern void update_stderr_ui_file (struct ui_file *ui_file);
+
+
 /* Open NAME returning an STDIO based UI_FILE.  */
 extern struct ui_file *gdb_fopen (const char *name, const char *mode);
 
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-23  2:20                                           ` Yao Qi
@ 2013-08-23 13:38                                             ` Joel Brobecker
  2013-08-27 20:39                                             ` Pedro Alves
  1 sibling, 0 replies; 44+ messages in thread
From: Joel Brobecker @ 2013-08-23 13:38 UTC (permalink / raw)
  To: Yao Qi; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches

> > Just a question: Would it be possible to emulate line buffering
> > as well? In my experience, the size of the buffer is fairly big,
> > and causes the output to appear only at fairly large intervals,
> > and in fairly large blobs.
> 
> I am not sure.  The goal of this patch is to keep the order of outputs
> from stdout and stderr correct, so I don't think much on emulating
> line buffering for stdout.  IMO, it can be addressed by a separated
> patch.

True! Let's keep the extra complexity out.

Thanks for doing this,
-- 
Joel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-23  2:20                                           ` Yao Qi
  2013-08-23 13:38                                             ` Joel Brobecker
@ 2013-08-27 20:39                                             ` Pedro Alves
  2013-08-28  7:23                                               ` Yao Qi
  1 sibling, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-08-27 20:39 UTC (permalink / raw)
  To: Yao Qi; +Cc: Joel Brobecker, Eli Zaretskii, gdb-patches

On 08/23/2013 03:19 AM, Yao Qi wrote:
> 2013-08-23  Yao Qi  <yao@codesourcery.com>
> 
> 	* main.c (captured_main) [__MINGW32__]: Set stderr unbuffered.
> 	and all update_stderr_ui_file.
> 	* ui-file.c (stderr_file_write): New function.
> 	(stderr_file_fputs): New function.
> 	(update_stderr_ui_file): New function.
> 	* ui-file.h (update_stderr_ui_file): Declare.
> ---
>  gdb/main.c    |   20 ++++++++++++++++++++
>  gdb/ui-file.c |   32 ++++++++++++++++++++++++++++++++
>  gdb/ui-file.h |    3 +++
>  3 files changed, 55 insertions(+), 0 deletions(-)
> 
> diff --git a/gdb/main.c b/gdb/main.c
> index 1c240e4..332df9e 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -375,8 +375,28 @@ captured_main (void *data)
>    saved_command_line[0] = '\0';
>    instream = stdin;
>  
> +#ifdef __MINGW32__
> +  /* Ensure stderr is unbuffered.  A Cygwin pty or pipe is implemented
> +     as a Windows pipe, and Windows buffers on pipes.  */
> +  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
> +#endif
> +
>    gdb_stdout = stdio_fileopen (stdout);
>    gdb_stderr = stdio_fileopen (stderr);
> +#ifdef __MINGW32__
> +  /* There is no real line-buffering on Windows, see
> +     http://msdn.microsoft.com/en-us/library/86cebhfs%28v=vs.71%29.aspx
> +     so the stdout is either fully-buffered or non-buffered.  We can't
> +     make stdout non-buffered, because of two concerns,
> +     1.  non-buffering hurts performance,
> +     2.  non-buffering may change GDB's behavior when it is interacting
> +     with front-end, such as Emacs.
> +
> +     We decided to leave stdout as fully buffered, but flush it first
> +     when something is written to stderr.  */
> +  update_stderr_ui_file (gdb_stderr);
> +#endif
> +
>    gdb_stdlog = gdb_stderr;	/* for moment */
>    gdb_stdtarg = gdb_stderr;	/* for moment */
>    gdb_stdin = stdio_fileopen (stdin);
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index cf5a86d..4b26b78 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -654,6 +654,38 @@ stdio_file_fseek (struct ui_file *file, long offset, int whence)
>    return fseek (stdio->file, offset, whence);
>  }
>  
> +/* This is the implementation of ui_file method to_write for stderr.
> +   gdb_stdout is flushed before writing to gdb_stderr.  */
> +
> +static void
> +stderr_file_write (struct ui_file *file, const char *buf, long length_buf)
> +{
> +  gdb_flush (gdb_stdout);
> +  stdio_file_write (file, buf, length_buf);
> +}
> +
> +/* This is the implementation of ui_file method to_fputs for stderr.
> +   gdb_stdout is flushed before writing to gdb_stderr.  */
> +
> +static void
> +stderr_file_fputs (const char *linebuffer, struct ui_file *file)
> +{
> +  gdb_flush (gdb_stdout);
> +  stdio_file_fputs (linebuffer, file);
> +}
> +
> +/* Overwrite UI_FILE's methods 'to_write' and 'to_fputs' by function
> +   in which gdb_stdout is flushed.  */
> +
> +void
> +update_stderr_ui_file (struct ui_file *ui_file)
> +{
> +  /* Method 'to_write_async_safe' is not overwritten, because it is
> +     only used on linux host.  */

Better say that "it is not used on Windows hosts".  But I think
it'd be better even to say:

  /* Method 'to_write_async_safe' is not overwritten, because
     there's no way to flushing a stream in an async-safe manner.
     Fortunately, it doesn't really matter, because:
       - that method is only used for printing internal debug output
	 from signal handlers.
       - Windows hosts don't have a concept of async-safeness.  Signal
	 handlers run in a separate thread, so they can call
         the regular non-async-safe output routines freely.  */

I just noticed there's another place that creates gdb_stderr from
stderr in event-top.c:gdb_setup_readline.  That's called from
several interpreter_resume methods.  Though, I don't really understand
why that isn't constantly leaking gdb_stderr objects, cause it just
looks like it does.  Ah, gdb_disable_readline.  Urgh.:

/* Disable command input through the standard CLI channels.  Used in
   the suspend proc for interpreters that use the standard gdb readline
   interface, like the cli & the mi.  */
void
gdb_disable_readline (void)
{
  /* FIXME - It is too heavyweight to delete and remake these every
     time you run an interpreter that needs readline.  It is probably
     better to have the interpreters cache these, which in turn means
     that this needs to be moved into interpreter specific code.  */

#if 0
  ui_file_delete (gdb_stdout);
  ui_file_delete (gdb_stderr);
  gdb_stdlog = NULL;
  gdb_stdtarg = NULL;
  gdb_stdtargerr = NULL;
#endif

  rl_callback_handler_remove ();
  delete_file_handler (input_fd);
}

This means that after an "interpreter-exec" sequence, the
update_stderr_ui_file hack is lost...

Even if that were fixed, thinking ahead in terms of C++, I think it'd
be better to not expose to common code outside ui-file.c that you can
tweak ui_file's virtual methods at runtime like that, but instead to
hide that in ui-file.c itself.  So e.g., we'd have a new stderr_fileopen
method, and then both places that create gdb_stderr would use it, like:

-gdb_stderr = stdio_fileopen (stderr);
+gdb_stderr = stderr_fileopen ();

We could also that whole set of Windows-specific comments there
too.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-27 20:39                                             ` Pedro Alves
@ 2013-08-28  7:23                                               ` Yao Qi
  2013-08-28  9:39                                                 ` Pedro Alves
  0 siblings, 1 reply; 44+ messages in thread
From: Yao Qi @ 2013-08-28  7:23 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Joel Brobecker, Eli Zaretskii, gdb-patches

On 08/28/2013 04:39 AM, Pedro Alves wrote:
> Better say that "it is not used on Windows hosts".  But I think
> it'd be better even to say:
> 
>    /* Method 'to_write_async_safe' is not overwritten, because
>       there's no way to flushing a stream in an async-safe manner.
>       Fortunately, it doesn't really matter, because:
>         - that method is only used for printing internal debug output
> 	 from signal handlers.
>         - Windows hosts don't have a concept of async-safeness.  Signal
> 	 handlers run in a separate thread, so they can call
>           the regular non-async-safe output routines freely.  */

OK.

[...]
> hide that in ui-file.c itself.  So e.g., we'd have a new stderr_fileopen
> method, and then both places that create gdb_stderr would use it, like:
> 
> -gdb_stderr = stdio_fileopen (stderr);
> +gdb_stderr = stderr_fileopen ();
> 
> We could also that whole set of Windows-specific comments there
> too.

Right.  Function stderr_fileopen is added in the updated patch.
What do you think?

-- 
Yao (齐尧)

gdb:

2013-08-28  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* event-top.c (gdb_setup_readline): Call stderr_fileopen
	instead of stdio_fileopen.
	* main.c (captured_main) [__MINGW32__]: Set stderr unbuffered.
	Call stderr_fileopen instead of stdio_fileopen.
	* ui-file.c (stderr_file_write): New function.
	(stderr_file_fputs): New function.
	(stderr_fileopen): New function.
	* ui-file.h (stderr_fileopen): Declare.
---
 gdb/event-top.c |    2 +-
 gdb/main.c      |    9 ++++++++-
 gdb/ui-file.c   |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/ui-file.h   |    3 +++
 4 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index f00ab7d..f1d55b3 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -955,7 +955,7 @@ gdb_setup_readline (void)
      time.  */
   if (!batch_silent)
     gdb_stdout = stdio_fileopen (stdout);
-  gdb_stderr = stdio_fileopen (stderr);
+  gdb_stderr = stderr_fileopen ();
   gdb_stdlog = gdb_stderr;  /* for moment */
   gdb_stdtarg = gdb_stderr; /* for moment */
   gdb_stdtargerr = gdb_stderr; /* for moment */
diff --git a/gdb/main.c b/gdb/main.c
index 1c240e4..11f4b03 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -375,8 +375,15 @@ captured_main (void *data)
   saved_command_line[0] = '\0';
   instream = stdin;
 
+#ifdef __MINGW32__
+  /* Ensure stderr is unbuffered.  A Cygwin pty or pipe is implemented
+     as a Windows pipe, and Windows buffers on pipes.  */
+  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+#endif
+
   gdb_stdout = stdio_fileopen (stdout);
-  gdb_stderr = stdio_fileopen (stderr);
+  gdb_stderr = stderr_fileopen ();
+
   gdb_stdlog = gdb_stderr;	/* for moment */
   gdb_stdtarg = gdb_stderr;	/* for moment */
   gdb_stdin = stdio_fileopen (stdin);
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index cf5a86d..d29b8da 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -654,6 +654,60 @@ stdio_file_fseek (struct ui_file *file, long offset, int whence)
   return fseek (stdio->file, offset, whence);
 }
 
+/* This is the implementation of ui_file method to_write for stderr.
+   gdb_stdout is flushed before writing to gdb_stderr.  */
+
+static void
+stderr_file_write (struct ui_file *file, const char *buf, long length_buf)
+{
+  gdb_flush (gdb_stdout);
+  stdio_file_write (file, buf, length_buf);
+}
+
+/* This is the implementation of ui_file method to_fputs for stderr.
+   gdb_stdout is flushed before writing to gdb_stderr.  */
+
+static void
+stderr_file_fputs (const char *linebuffer, struct ui_file *file)
+{
+  gdb_flush (gdb_stdout);
+  stdio_file_fputs (linebuffer, file);
+}
+
+/* Create a ui_file from stderr.  */
+
+struct ui_file *
+stderr_fileopen (void)
+{
+  struct ui_file *ui_file = stdio_fileopen (stderr);
+
+#ifdef __MINGW32__
+  /* There is no real line-buffering on Windows, see
+     http://msdn.microsoft.com/en-us/library/86cebhfs%28v=vs.71%29.aspx
+     so the stdout is either fully-buffered or non-buffered.  We can't
+     make stdout non-buffered, because of two concerns,
+     1.  non-buffering hurts performance,
+     2.  non-buffering may change GDB's behavior when it is interacting
+     with front-end, such as Emacs.
+
+     We decided to leave stdout as fully buffered, but flush it first
+     when something is written to stderr.  */
+
+  /* Method 'to_write_async_safe' is not overwritten, because there's
+     no way to flushing a stream in an async-safe manner.
+     Fortunately, it doesn't really matter, because:
+     - that method is only used for printing internal debug output
+       from signal handlers.
+     - Windows hosts don't have a concept of async-safeness.  Signal
+       handlers run in a separate thread, so they can call
+       the regular non-async-safe output routines freely.  */
+  set_ui_file_write (ui_file, stderr_file_write);
+  set_ui_file_fputs (ui_file, stderr_file_fputs);
+#endif
+
+  return ui_file;
+}
+
 /* Like fdopen().  Create a ui_file from a previously opened FILE.  */
 
 struct ui_file *
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 9fef68c..dca800f 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -129,6 +129,9 @@ extern struct ui_file *mem_fileopen (void);
 /* Open/create a STDIO based UI_FILE using the already open FILE.  */
 extern struct ui_file *stdio_fileopen (FILE *file);
 
+extern struct ui_file *stderr_fileopen (void);
+
+
 /* Open NAME returning an STDIO based UI_FILE.  */
 extern struct ui_file *gdb_fopen (const char *name, const char *mode);
 
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-28  7:23                                               ` Yao Qi
@ 2013-08-28  9:39                                                 ` Pedro Alves
  2013-08-28 12:25                                                   ` Yao Qi
  0 siblings, 1 reply; 44+ messages in thread
From: Pedro Alves @ 2013-08-28  9:39 UTC (permalink / raw)
  To: Yao Qi; +Cc: Joel Brobecker, Eli Zaretskii, gdb-patches

On 08/28/2013 08:22 AM, Yao Qi wrote:

> +/* Create a ui_file from stderr.  */
> +

I think this should be in the header....

> +struct ui_file *
> +stderr_fileopen (void)
> +{

> @@ -129,6 +129,9 @@ extern struct ui_file *mem_fileopen (void);
>  /* Open/create a STDIO based UI_FILE using the already open FILE.  */
>  extern struct ui_file *stdio_fileopen (FILE *file);
>
> +extern struct ui_file *stderr_fileopen (void);
> +
> +

... as the other functions (seen from context) are documented here
too.

>  /* Open NAME returning an STDIO based UI_FILE.  */
>  extern struct ui_file *gdb_fopen (const char *name, const char *mode);



> +
> +  /* Method 'to_write_async_safe' is not overwritten, because there's
> +     no way to flushing a stream in an async-safe manner.

Sorry, my own fault -- should read "to flush".

This is OK with those fixes.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] Unbuffer stdout and stderr on windows
  2013-08-28  9:39                                                 ` Pedro Alves
@ 2013-08-28 12:25                                                   ` Yao Qi
  0 siblings, 0 replies; 44+ messages in thread
From: Yao Qi @ 2013-08-28 12:25 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Joel Brobecker, Eli Zaretskii, gdb-patches

On 08/28/2013 05:38 PM, Pedro Alves wrote:
> 
> 
>> >+
>> >+  /* Method 'to_write_async_safe' is not overwritten, because there's
>> >+     no way to flushing a stream in an async-safe manner.
> Sorry, my own fault -- should read "to flush".
> 

Never mind.  Fixed.

> This is OK with those fixes.

Patch below is what I committed.  Note that I wrap up stderr_file_write
and stderr_file_fputs by #ifdef __MINGW32__ and #endif, to fix the
warning on unused functions on non-windows host.

-- 
Yao (齐尧)

gdb:

2013-08-28  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* event-top.c (gdb_setup_readline): Call stderr_fileopen
	instead of stdio_fileopen.
	* main.c (captured_main) [__MINGW32__]: Set stderr unbuffered.
	.Call stderr_fileopen instead of stdio_fileopen.
	* ui-file.c [__MINGW32__] (stderr_file_write): New function.
	[__MINGW32__] (stderr_file_fputs): New function.
	(stderr_fileopen): New function.
	* ui-file.h (stderr_fileopen): Declare.
---
 gdb/event-top.c |    2 +-
 gdb/main.c      |    9 ++++++++-
 gdb/ui-file.c   |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/ui-file.h   |    4 ++++
 4 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index f00ab7d..f1d55b3 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -955,7 +955,7 @@ gdb_setup_readline (void)
      time.  */
   if (!batch_silent)
     gdb_stdout = stdio_fileopen (stdout);
-  gdb_stderr = stdio_fileopen (stderr);
+  gdb_stderr = stderr_fileopen ();
   gdb_stdlog = gdb_stderr;  /* for moment */
   gdb_stdtarg = gdb_stderr; /* for moment */
   gdb_stdtargerr = gdb_stderr; /* for moment */
diff --git a/gdb/main.c b/gdb/main.c
index 1c240e4..11f4b03 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -375,8 +375,15 @@ captured_main (void *data)
   saved_command_line[0] = '\0';
   instream = stdin;
 
+#ifdef __MINGW32__
+  /* Ensure stderr is unbuffered.  A Cygwin pty or pipe is implemented
+     as a Windows pipe, and Windows buffers on pipes.  */
+  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+#endif
+
   gdb_stdout = stdio_fileopen (stdout);
-  gdb_stderr = stdio_fileopen (stderr);
+  gdb_stderr = stderr_fileopen ();
+
   gdb_stdlog = gdb_stderr;	/* for moment */
   gdb_stdtarg = gdb_stderr;	/* for moment */
   gdb_stdin = stdio_fileopen (stdin);
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index cf5a86d..b9d3e53 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -654,6 +654,60 @@ stdio_file_fseek (struct ui_file *file, long offset, int whence)
   return fseek (stdio->file, offset, whence);
 }
 
+#ifdef __MINGW32__
+/* This is the implementation of ui_file method to_write for stderr.
+   gdb_stdout is flushed before writing to gdb_stderr.  */
+
+static void
+stderr_file_write (struct ui_file *file, const char *buf, long length_buf)
+{
+  gdb_flush (gdb_stdout);
+  stdio_file_write (file, buf, length_buf);
+}
+
+/* This is the implementation of ui_file method to_fputs for stderr.
+   gdb_stdout is flushed before writing to gdb_stderr.  */
+
+static void
+stderr_file_fputs (const char *linebuffer, struct ui_file *file)
+{
+  gdb_flush (gdb_stdout);
+  stdio_file_fputs (linebuffer, file);
+}
+#endif
+
+struct ui_file *
+stderr_fileopen (void)
+{
+  struct ui_file *ui_file = stdio_fileopen (stderr);
+
+#ifdef __MINGW32__
+  /* There is no real line-buffering on Windows, see
+     http://msdn.microsoft.com/en-us/library/86cebhfs%28v=vs.71%29.aspx
+     so the stdout is either fully-buffered or non-buffered.  We can't
+     make stdout non-buffered, because of two concerns,
+     1.  non-buffering hurts performance,
+     2.  non-buffering may change GDB's behavior when it is interacting
+     with front-end, such as Emacs.
+
+     We decided to leave stdout as fully buffered, but flush it first
+     when something is written to stderr.  */
+
+  /* Method 'to_write_async_safe' is not overwritten, because there's
+     no way to flush a stream in an async-safe manner.  Fortunately,
+     it doesn't really matter, because:
+     - that method is only used for printing internal debug output
+       from signal handlers.
+     - Windows hosts don't have a concept of async-safeness.  Signal
+       handlers run in a separate thread, so they can call
+       the regular non-async-safe output routines freely.  */
+  set_ui_file_write (ui_file, stderr_file_write);
+  set_ui_file_fputs (ui_file, stderr_file_fputs);
+#endif
+
+  return ui_file;
+}
+
 /* Like fdopen().  Create a ui_file from a previously opened FILE.  */
 
 struct ui_file *
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 9fef68c..ba0a908 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -129,6 +129,10 @@ extern struct ui_file *mem_fileopen (void);
 /* Open/create a STDIO based UI_FILE using the already open FILE.  */
 extern struct ui_file *stdio_fileopen (FILE *file);
 
+/* Create a ui_file from stderr.  */
+extern struct ui_file *stderr_fileopen (void);
+
+
 /* Open NAME returning an STDIO based UI_FILE.  */
 extern struct ui_file *gdb_fopen (const char *name, const char *mode);
 
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2013-08-28 12:25 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-22  3:07 [PATCH] Unbuffer stdout and stderr on windows Yao Qi
2013-07-22 15:41 ` Eli Zaretskii
2013-07-23  6:35   ` Yao Qi
2013-07-23 17:52     ` Eli Zaretskii
2013-07-29 19:26       ` Christopher Faylor
2013-07-29 19:30         ` Eli Zaretskii
2013-07-29 19:51           ` Pedro Alves
2013-07-31  3:40             ` Christopher Faylor
2013-08-12 21:11               ` Joel Brobecker
2013-08-13 17:28                 ` Christopher Faylor
2013-08-13 18:08                 ` Eli Zaretskii
2013-08-14  0:05                   ` Joel Brobecker
2013-08-15 17:36                   ` Christopher Faylor
2013-08-15 17:44                     ` Eli Zaretskii
2013-08-15 17:59                       ` Christopher Faylor
2013-08-15 18:44                         ` Eli Zaretskii
2013-08-16 11:46                         ` Pedro Alves
2013-08-16 12:34                           ` Yao Qi
2013-08-16 13:20                             ` Eli Zaretskii
2013-08-16 13:37                             ` Pedro Alves
2013-08-16 14:03                               ` Eli Zaretskii
2013-08-16 14:21                                 ` Pedro Alves
2013-08-16 14:57                                   ` Eli Zaretskii
2013-08-16 15:10                                     ` Pedro Alves
2013-08-16 15:24                                       ` Pedro Alves
2013-08-16 15:43                                         ` Eli Zaretskii
2013-08-16 16:41                                           ` Christopher Faylor
2013-08-16 15:41                                       ` Eli Zaretskii
2013-08-22  6:14                                       ` Yao Qi
2013-08-22 14:18                                         ` Joel Brobecker
2013-08-23  2:20                                           ` Yao Qi
2013-08-23 13:38                                             ` Joel Brobecker
2013-08-27 20:39                                             ` Pedro Alves
2013-08-28  7:23                                               ` Yao Qi
2013-08-28  9:39                                                 ` Pedro Alves
2013-08-28 12:25                                                   ` Yao Qi
2013-08-16 13:17                           ` Eli Zaretskii
2013-08-16 13:30                             ` Pedro Alves
2013-08-16 13:42                               ` Eli Zaretskii
2013-08-16 14:13                                 ` Pedro Alves
2013-08-16 14:44                                   ` Eli Zaretskii
2013-08-16 15:05                                     ` Pedro Alves
2013-08-16 15:13                                       ` Eli Zaretskii
2013-07-29 19:30         ` Christopher Faylor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).