From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10843 invoked by alias); 27 Aug 2013 20:39:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 10833 invoked by uid 89); 27 Aug 2013 20:39:48 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 27 Aug 2013 20:39:48 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.1 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r7RKdeUu026827 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 27 Aug 2013 16:39:40 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r7RKdcoR024908; Tue, 27 Aug 2013 16:39:39 -0400 Message-ID: <521D0E8A.80303@redhat.com> Date: Tue, 27 Aug 2013 20:39:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Yao Qi CC: Joel Brobecker , Eli Zaretskii , gdb-patches@sourceware.org Subject: Re: [PATCH] Unbuffer stdout and stderr on windows References: <83eh9uonlg.fsf@gnu.org> <20130815175940.GD6955@ednor.casa.cgf.cx> <520E1109.7000304@redhat.com> <520E1C34.2000907@codesourcery.com> <520E2B13.8020706@redhat.com> <83r4dtn35q.fsf@gnu.org> <520E357E.6080803@redhat.com> <83mwohn0nj.fsf@gnu.org> <520E40CD.7080604@redhat.com> <5215AC28.3030506@codesourcery.com> <20130822141756.GB5221@adacore.com> <5216C6B0.90906@codesourcery.com> In-Reply-To: <5216C6B0.90906@codesourcery.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-08/txt/msg00801.txt.bz2 On 08/23/2013 03:19 AM, Yao Qi wrote: > 2013-08-23 Yao Qi > > * 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