This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v4 2/2] Eliminate make_cleanup_ui_file_delete / make ui_file a class hierarchy
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;
};
> Otherwise looks good to me.
>
> Great cleanup. Thanks!
Thanks!
--
Pedro Alves