From patchwork Wed Feb 1 22:49:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 19087 Received: (qmail 26569 invoked by alias); 1 Feb 2017 22:49:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 26548 invoked by uid 89); 1 Feb 2017 22:49:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=BAYES_50, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=586, deferring, flushes, aspx X-HELO: mx1.redhat.com 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; Wed, 01 Feb 2017 22:49:05 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BEA92369C4; Wed, 1 Feb 2017 22:49:05 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v11Mn4F9018856; Wed, 1 Feb 2017 17:49:04 -0500 Subject: Re: [PATCH v4 2/2] Eliminate make_cleanup_ui_file_delete / make ui_file a class hierarchy To: Luis Machado , gdb-patches@sourceware.org References: <1485909045-30285-1-git-send-email-palves@redhat.com> <1485909045-30285-3-git-send-email-palves@redhat.com> From: Pedro Alves Message-ID: <5b7dfd6c-483e-618b-0d25-7f5b12ad32de@redhat.com> Date: Wed, 1 Feb 2017 22:49:02 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: On 02/01/2017 05:36 PM, Luis Machado wrote: >> +typedef std::unique_ptr stdio_file_up; >> + >> +/* Like stdio_file, but specifically for stderr. >> + >> + This exists because there is no real line-buffering on Windows, see >> + >> + 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 a front-end, such as Emacs. >> + >> + We leave stdout as fully buffered, but flush it first when >> + something is written to stderr. >> + >> + Note the the 'write_async_safe' method is not overwritten, because > > > Extra "the". > > Did you mean overridden instead of overwritten? Right, fixed. The existing comment this is being moved from says "overwritten", and I missed updating it. ("overwritten" made some sense in current master, I guess, since to "override" a method currently you "overwrite" a function pointer.) >> +class stderr_file : public stdio_file >> +{ >> +public: >> + explicit stderr_file (FILE *stream); >> >> -/* Create/open a memory based file. Can be used as a scratch buffer >> - for collecting output. */ >> -extern struct ui_file *mem_fileopen (void); >> + /* Flushes gdb_stdout before writing to the underlying stream. */ >> + void write (const char *buf, long length_buf) override; >> > > I noticed the above declaration and ... > >> + /* Flushes gdb_stdout before writing to the underlying stream. */ >> + void puts (const char *linebuffer) override; > > ... the above declaration both have the same documentation. Do they > accomplish the same? They both flush gdb_stdout before deferring to the stdio_file (the superclass) for the actual writing/outputting. "puts" exists as a separate method because for some ui_file types it's more efficient to call some available puts-like function (e.g. tui_puts), than having the puts method always always call the write method, which requires a strlen call. Would this help? gdb/ui-file.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) > Otherwise looks good to me. > > Great cleanup. Thanks! Thanks! diff --git a/gdb/ui-file.h b/gdb/ui-file.h index fc70417..d64cdce 100644 --- a/gdb/ui-file.h +++ b/gdb/ui-file.h @@ -58,6 +58,8 @@ public: virtual void write_async_safe (const char *buf, long length_buf) { gdb_assert_not_reached ("write_async_safe"); } + /* Some ui_files override this to provide a efficient implementation + that avoids the strlen. */ virtual void puts (const char *str) { this->write (str, strlen (str)); } @@ -227,10 +229,9 @@ class stderr_file : public stdio_file public: explicit stderr_file (FILE *stream); - /* Flushes gdb_stdout before writing to the underlying stream. */ + /* Override the output routines to flush gdb_stdout before deferring + to stdio_file for the actual outputting. */ void write (const char *buf, long length_buf) override; - - /* Flushes gdb_stdout before writing to the underlying stream. */ void puts (const char *linebuffer) override; };