This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v4 2/2] Eliminate make_cleanup_ui_file_delete / make ui_file a class hierarchy


On 02/01/2017 04:49 PM, Pedro Alves wrote:
On 02/01/2017 05:36 PM, Luis Machado wrote:

+typedef std::unique_ptr<stdio_file> stdio_file_up;
+
+/* Like stdio_file, but specifically for stderr.
+
+   This exists because 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 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(-)

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;
 };


That looks better. Thanks.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]