Re: [PATCH] Unbuffer stdout and stderr on windows

[Joel Brobecker <brobecker@adacore.com>]

> 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