This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 5/5] Eliminate make_cleanup_ui_file_delete / make ui_file a class hierarchy
On 01/17/2017 07:57 PM, Simon Marchi wrote:
> I've only looked at ui-file.h/c so far, but overall it looks really nice.
>
> On 2017-01-16 20:39, Pedro Alves wrote:
>> -static void
>> -null_file_fputs (const char *buf, struct ui_file *file)
>> -{
>> - if (file->to_write == null_file_write)
>> - /* Both the write and fputs methods are null. Discard the
>> - request. */
>> - return;
>> - else
>> - {
>> - /* The write method was implemented, use that. */
>> - file->to_write (file, buf, strlen (buf));
>> - }
>> -}
>> +
>
> Is the insertion of a form feed character on purpose? I know it's
> mentioned in the GNU coding style, but I was wondering if they were
> useful to anybody today.
I sometimes find them useful to visually split sections, and
to navigate them, though I don't usually remember to add them.
In this case, there were already form feeds in
place, and I guess they get more visible because there's so
much less code in the file, or I copy/pasted around or
something. I've now gone ever them and made sure they're at
appropriate places (i.e., separating the methods of each class).
>
>> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
>> index 4ad3940..f1951e4 100644
>> --- a/gdb/ui-file.h
>> +++ b/gdb/ui-file.h
>> @@ -24,74 +24,86 @@ struct ui_file;
>
> The forward declarations at the beginning of the file can go.
>
Indeed, fixed.
>>
>> #include <string>
>>
>> -/* Create a generic ui_file object with null methods. */
>> -
>> -extern struct ui_file *ui_file_new (void);
>> -
>> -/* Override methods used by specific implementations of a UI_FILE
>> - object. */
>> -
>> -typedef void (ui_file_flush_ftype) (struct ui_file *stream);
>> -extern void set_ui_file_flush (struct ui_file *stream,
>> - ui_file_flush_ftype *flush);
>> -
>> -/* NOTE: Both fputs and write methods are available. Default
>> - implementations that mapping one onto the other are included. */
>> -typedef void (ui_file_write_ftype) (struct ui_file *stream,
>> - const char *buf, long length_buf);
>> -extern void set_ui_file_write (struct ui_file *stream,
>> - ui_file_write_ftype *fputs);
>> -
>> -typedef void (ui_file_fputs_ftype) (const char *, struct ui_file
>> *stream);
>> -extern void set_ui_file_fputs (struct ui_file *stream,
>> - ui_file_fputs_ftype *fputs);
>> -
>> -/* This version of "write" is safe for use in signal handlers.
>> - It's not guaranteed that all existing output will have been
>> - flushed first.
>> - Implementations are also free to ignore some or all of the request.
>> - fputs_async is not provided as the async versions are rarely used,
>> - no point in having both for a rarely used interface. */
>> -typedef void (ui_file_write_async_safe_ftype)
>> - (struct ui_file *stream, const char *buf, long length_buf);
>> -extern void set_ui_file_write_async_safe
>> - (struct ui_file *stream, ui_file_write_async_safe_ftype
>> *write_async_safe);
>> -
>> -typedef long (ui_file_read_ftype) (struct ui_file *stream,
>> - char *buf, long length_buf);
>> -extern void set_ui_file_read (struct ui_file *stream,
>> - ui_file_read_ftype *fread);
>> -
>> -typedef int (ui_file_isatty_ftype) (struct ui_file *stream);
>> -extern void set_ui_file_isatty (struct ui_file *stream,
>> - ui_file_isatty_ftype *isatty);
>> -
>> -typedef void (ui_file_rewind_ftype) (struct ui_file *stream);
>> -extern void set_ui_file_rewind (struct ui_file *stream,
>> - ui_file_rewind_ftype *rewind);
>> -
>> typedef void (ui_file_put_method_ftype) (void *object, const char
>> *buffer,
>> long length_buffer);
>
> This one can go too (actually you've marked it as deleted in the
> changelog).
Fixed.
>
>> -typedef void (ui_file_put_ftype) (struct ui_file *stream,
>> - ui_file_put_method_ftype *method,
>> - void *context);
>> -extern void set_ui_file_put (struct ui_file *stream,
>> ui_file_put_ftype *put);
>>
>> -typedef void (ui_file_delete_ftype) (struct ui_file * stream);
>> -extern void set_ui_file_data (struct ui_file *stream, void *data,
>> - ui_file_delete_ftype *to_delete);
>> +/* The ui_file base class. */
>>
>> -typedef int (ui_file_fseek_ftype) (struct ui_file *stream, long offset,
>> - int whence);
>> -extern void set_ui_file_fseek (struct ui_file *stream,
>> - ui_file_fseek_ftype *fseek_ptr);
>> +class ui_file
>> +{
>> +public:
>> + ui_file ();
>> + virtual ~ui_file ();
>>
>> -extern void *ui_file_data (struct ui_file *file);
>> + /* Public non-virtual API. */
>>
>> + void printf (const char *, ...) ATTRIBUTE_PRINTF (2, 3);
>>
>> -extern void gdb_flush (struct ui_file *);
>> + /* Print a string whose delimiter is QUOTER. Note that these
>> + routines should only be call for printing things which are
>
> call -> called?
>
Fixed.
>> + /* Rewind local buffer. I.e., clear it. */
>> + virtual void rewind ()
>> + {}
>
> I'm not sure about the implications, but instinctively I would've made
> rewind in the base class throw an exception by default, instead of
> making the derived types throw one if they don't support rewind.
> Actually, except for the null_file, I don't really see when you would
> want rewind to be a no-op. If the rewind call returns but does not
> actually rewind, isn't it lying to the client code?
Yes, indeed it's a lie, but it's not really a new lie. :-) It's similar to
how the existing code works. Before the patch, write_buffer_on doesn't
exist, but what it replaces, the "put" callback, behaved similarly -- only
mem_fileopen, TUI and guile ui-files implement the put method,
all others leave it set to null_file_put (the default).
"write/write_buffer_on", like "put" were only meant for
locally buffered files (like mem_fileopen/tui_sfileopen).
ioscm_file_port might have only implemented put because the
method was there in ui_file to implement.
It's a bit of a bogus design, but it didn't seem easy to fix.
The asserts were me trying to figure out where are the calls to
write_buffer_on / rewind coming from. I meant to come back to this
before posting, but then ended up posting this sooner than I
thought, and forgot to remove the asserts. Anyway, I took another
look at all this today, and I have a proposal.
After my patch, the ui_file::write_buffer_on method is only
used here, in the MI's ui_out:
void
mi_ui_out::put (ui_file *stream)
{
ui_file *outstream = m_streams.back ();
outstream->write_buffer_on (*stream);
outstream->rewind ();
}
When I was originally writing this series a couple months ago, I tried
to come up with a way to get here with the m_streams vector containing
some ui_file at the top of the stack that is not the ui_out's original
stream_file. And I think I had found some, around enabling
MI logging, and so to preserve behavior, I was planning on leaving
both the write_buffer_on and rewind methods as no-ops for
most ui_files, thinking that to fix this we're need a larger
redesign to the CLI/MI logging code (which looks kind of horrible)
that would be better done on top.
I redid the experiment / investigation here, and it looks
the only way to reach that method with 'outstream' being something
other than the ui_out's original string_file, is with this
this sequence of MI commands:
-gdb-set logging on
-gdb-set logging redirect on
This reaches mi_ui_out::put with 'outstream' being a
stdio_file, and triggers the new assertion in stdio_file:
void rewind () override
{ gdb_assert_not_reached ("can't rewind a stdio file"); }
#0 0x00000000007ad1b4 in internal_error(char const*, int, char const*, ...) (file=0x1145b50 "/home/pedro/gdb/mygit/cxx-convertion/src/gdb/ui-file.h", line=190, fmt=0x1145b44 "%s: %s") at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/common/errors.c:54
#1 0x0000000000639b77 in stdio_file::rewind() (this=0x335d7e0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/ui-file.h:190
#2 0x0000000000634ee4 in mi_ui_out::put(ui_file*) (this=0x2a010e0, stream=0x335d7e0)
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/mi/mi-out.c:255
#3 0x000000000063519f in mi_out_put(ui_out*, ui_file*) (uiout=0x2a010e0, stream=0x335d7e0)
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/mi/mi-out.c:314
#4 0x0000000000631b45 in captured_mi_execute_command(ui_out*, mi_parse*) (uiout=0x2a010e0, context=0x335d2e0)
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/mi/mi-main.c:2007
#5 0x000000000063203d in mi_execute_command(char const*, int) (cmd=0x335d370 "-gdb-set logging redirect on", from_tty=1)
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/mi/mi-main.c:2157
#6 0x000000000062b0f4 in mi_execute_command_wrapper(char const*) (cmd=0x335d370 "-gdb-set logging redirect on")
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/mi/mi-interp.c:289
#7 0x000000000062b180 in mi_execute_command_input_handler(char*) (cmd=0x335d370 "-gdb-set logging redirect on")
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/mi/mi-interp.c:319
#8 0x00000000007b8e37 in gdb_readline_no_editing_callback(void*) (client_data=0x2366b10)
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-top.c:855
#9 0x00000000007b87a7 in stdin_event_handler(int, void*) (error=0, client_data=0x2366b10)
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-top.c:518
#10 0x00000000007b6dcc in handle_file_event(file_handler*, int) (file_ptr=0x2e4fc90, ready_mask=1)
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:733
#11 0x00000000007b736f in gdb_wait_for_event(int) (block=1) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:859
#12 0x00000000007b61ec in gdb_do_one_event() () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:347
#13 0x00000000007b6224 in start_event_loop() () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:371
#14 0x0000000000827f16 in captured_command_loop(void*) (data=0x0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:325
#15 0x00000000007b9c8e in catch_errors(int (*)(void*), void*, char*, return_mask) (func=0x827ed4 <captured_command_loop(void*)>, func_args=0x0, errstring=0x11df891 "", mask=RETURN_MASK_ALL) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/exceptions.c:236
#16 0x000000000082928b in captured_main(void*) (data=0x7fffffffd820) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:1148
During symbol reading, cannot get low and high bounds for subprogram DIE at 24065.
#17 0x00000000008292b4 in gdb_main(captured_main_args*) (args=0x7fffffffd820) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:1158
#18 0x0000000000412d4d in main(int, char**) (argc=4, argv=0x7fffffffd928) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/gdb.c:32
Now, looking again with fresh eyes, I now believe that the
only reason this assert triggers, is because the implementation
of that sequence of commands for MI is plain buggy. If
you do it with current master, GDB crashes!
Program received signal SIGSEGV, Segmentation fault.
0x00000000008dd7bc in gdb_flush (file=0x2a097f0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/ui-file.c:194
194 file->to_flush (file);
(top-gdb) bt
#0 0x00000000008dd7bc in gdb_flush(ui_file*) (file=0x2a097f0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/ui-file.c:194
#1 0x00000000007b5f34 in gdb_wait_for_event(int) (block=0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:752
#2 0x00000000007b52b6 in gdb_do_one_event() () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:322
#3 0x00000000007b5362 in start_event_loop() () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:371
#4 0x000000000082704a in captured_command_loop(void*) (data=0x0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:325
#5 0x00000000007b8d7c in catch_errors(int (*)(void*), void*, char*, return_mask) (func=0x827008 <captured_command_loop(void*)>, func_args=0x0, errstring=0x11dee51 "", mask=RETURN_MASK_ALL) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/exceptions.c:236
#6 0x000000000082839b in captured_main(void*) (data=0x7fffffffd820) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:1148
During symbol reading, cannot get low and high bounds for subprogram DIE at 24065.
#7 0x00000000008283c4 in gdb_main(captured_main_args*) (args=0x7fffffffd820) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:1158
#8 0x0000000000412d4d in main(int, char**) (argc=4, argv=0x7fffffffd928) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/gdb.c:32
So it looks like the assert is catching a problem earlier.
Without the assert, gdb continues on with bogus state
and eventually crashes.
Going back to my patch, in the bad case, we get to:
void
mi_ui_out::put (ui_file *stream)
{
ui_file *outstream = m_streams.back ();
outstream->write_buffer_on (*stream);
outstream->rewind ();
}
with STREAM and OUTSTREAM pointing to the same thing...
This happens because we try to handle "set logging
redirect on" while logging is enabled:
https://sourceware.org/ml/gdb-patches/2010-08/msg00202.html
but get it wrong for MI. (Strictly speaking, MI logging support
was added after that patch, so it was the MI logging patch
that missed this.)
I find that code to handle on-the-fly redirect quite hard to
follow and reason about, along with the interp set_logging mechanism
and how CLI and MI hook into all this. I think the fix here should
simply be to stop trying to handle changing logging redirect on the fly,
and require turning logging off and on, like Jan's original patch
was proposing. I'll post a patch for that as follow up, to make
it clearer what I mean.
With that out of the way, it looks to me that we can assume
that mi_ui_put (and thus mi_ui_out::put) is only ever meant to
be called with output _not_ redirected, and thus we can simplify
things a bit further, by eliminating the ui_file
write_buffer_on / rewind methods, keeping it contained in
the MI code.
>
>>
>> -extern void ui_file_delete (struct ui_file *stream);
>> + /* Write contents of local buffer onto WRITE. */
>> + virtual void write_buffer_on (ui_file &where)
>
> Mismatch between parameter name and doc.
Fixed.
>
>> + /* Provide a few convenience methods with the same API as the
>> + underlying std::string. */
>> + const char *data () const { return m_string.data (); }
>> + const char *c_str () const { return m_string.c_str (); }
>> + size_t size () const { return m_string.size (); }
>> + bool empty () const;
>
> I am wondering why you didn't implement empty inline like the others,
> since it's very similar.
I have no idea. :-) Fixed.
>
>> +
>> +private:
>> + /* The internal buffer. */
>> + std::string m_string;
>> +};
>> +
>> +/* A ui_file implementation that maps directly onto <stdio.h>'s
>> + FILE. */
>> +
>> +class stdio_file : public ui_file
>> +{
>> +public:
>> + /* Create a ui_file from a previously opened FILE. If not CLOSE_P,
>> + then behave like like fdopen(). */
>
> "like like". It's also not very clear just saying "behave like
> fdopen()". Could it be a bit more explicit?
Indeed. I've added an intro comment explaining that a stdio_file
can either own the underlying file or not, and tweaked this comment
too.
>
>> + explicit stdio_file (FILE *file, bool close_p = false);
>> +
>> + /* Create an stdio_file that is not managing any file yet. Call
>> + open to actually open something. */
>> + stdio_file ();
>> +
>> + ~stdio_file () override;
>> +
>> + /* Open NAME in mode MODE. Returns true on success, false
>> + otherwise. Destroying the stdio_file closes the underlying FILE
>> + handle. */
>> + bool open (const char *name, const char *mode);
>
> What happens if you try to reuse that object by calling open twice?
> Does it leak an open file?
I hadn't handled that case, simply because we never reuse
a stdio_file. But it does sound likely to come up, so
I've made it close the previous file (if owned) now.
>
>> +class tee_file : public ui_file
>> +{
>> +public:
>> + /* Create a file which writes to both ONE and TWO. CLOSE_ONE and
>> + CLOSE_TWO indicate whether the original files should be closed
>> + when the new file is closed. */
>> + tee_file (ui_file *one, bool close_one,
>> + ui_file *two, bool close_two);
>> + ~tee_file () override;
>>
>> -/* Open/create a STDIO based UI_FILE using the already open FILE. */
>> -extern struct ui_file *stdio_fileopen (FILE *file);
>> + void write (const char *buf, long length_buf) override;
>> + void write_async_safe (const char *buf, long length_buf) override;
>> + void puts (const char *) override;
>>
>> -/* Likewise, for stderr-like streams. */
>> -extern struct ui_file *stderr_fileopen (FILE *file);
>> + bool isatty () override;
>> + void flush () override;
>>
>> + void rewind () override
>> + { gdb_assert_not_reached ("tee_file::rewind called\n"); }
>>
>> -/* Open NAME returning an STDIO based UI_FILE. */
>> -extern struct ui_file *gdb_fopen (const char *name, const char *mode);
>> +private:
>> + /* The two underlying ui_files, and whether they should each be
>> + closed on destruction. */
>> + struct ui_file *m_one, *m_two;
>
> Nit: I'm a big fan of dropping the struct keyword.
OK, I did:
git diff | grep "struct ui_file" | grep "^+"
and removed it in all the new code.
I've pushed the updated patch to a new users/palves/ui_file_v2
branch. I left the ui_file->write_buffer_on / ui_file->rewind methods
in place along with the gdb_asserts, just to make the patch
below, which applies on top, easier to understand. Should probably
either squash it, or remove the "rewind" asserts in the main patch.
I've pushed this one to the branch too. No regressions on x86_64 Fedora 23.
Let me know what you think.
>From ce661decce476bb18c5bb791ad4758c339d238cf Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 23 Jan 2017 15:28:28 +0000
Subject: [PATCH] Eliminate ui_file::write_buffer_on / ui_file::rewind
Leave string_file::rewind in place, but rename it to
string_file::clear, since it's now just a convenience around
std::string's clear.
---
gdb/cp-support.c | 2 +-
gdb/disasm.c | 4 ++--
gdb/guile/scm-ports.c | 7 -------
gdb/mi/mi-console.c | 2 +-
gdb/mi/mi-console.h | 6 ------
gdb/mi/mi-main.c | 4 ++--
gdb/mi/mi-out.c | 27 ++++++++++++++++-----------
gdb/mi/mi-out.h | 7 ++++++-
gdb/tui/tui-disasm.c | 4 ++--
gdb/ui-file.c | 18 ------------------
gdb/ui-file.h | 24 +-----------------------
gdb/ui-out.c | 2 +-
12 files changed, 32 insertions(+), 75 deletions(-)
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 30a58b1..1b0900e 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -327,7 +327,7 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
string and replace the top DEMANGLE_COMPONENT_QUAL_NAME
node. */
- buf.rewind ();
+ buf.clear ();
n = cp_comp_to_string (&newobj, 100);
if (n == NULL)
{
diff --git a/gdb/disasm.c b/gdb/disasm.c
index e29143ca..d84bd5c 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -240,7 +240,7 @@ gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
if (name != NULL)
xfree (name);
- stb.rewind ();
+ stb.clear ();
if (flags & DISASSEMBLY_RAW_INSN)
{
CORE_ADDR end_pc;
@@ -271,7 +271,7 @@ gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
size = gdbarch_print_insn (gdbarch, pc, di);
uiout->field_stream ("inst", stb);
- stb.rewind ();
+ stb.clear ();
do_cleanups (ui_out_chain);
uiout->text ("\n");
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 4f55e3c..fb3a47b 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -44,7 +44,6 @@ public:
explicit ioscm_file_port (SCM port);
void flush () override;
- void rewind () override;
void write (const char *buf, long length_buf) override;
private:
@@ -449,12 +448,6 @@ ioscm_file_port::flush ()
}
void
-ioscm_file_port::rewind ()
-{
- scm_truncate_file (m_port, 0);
-}
-
-void
ioscm_file_port::write (const char *buffer, long length_buffer)
{
scm_c_write (m_port, buffer, length_buffer);
diff --git a/gdb/mi/mi-console.c b/gdb/mi/mi-console.c
index e3071a6..218ffbc 100644
--- a/gdb/mi/mi-console.c
+++ b/gdb/mi/mi-console.c
@@ -81,7 +81,7 @@ mi_console_file::flush ()
gdb_flush (m_raw);
}
- m_buffer.rewind ();
+ m_buffer.clear ();
}
/* Change the underlying stream of the console directly; this is
diff --git a/gdb/mi/mi-console.h b/gdb/mi/mi-console.h
index 30569f1..a7962a8 100644
--- a/gdb/mi/mi-console.h
+++ b/gdb/mi/mi-console.h
@@ -41,12 +41,6 @@ public:
void puts (const char *) override;
- void write_buffer_on (ui_file &where) override
- { m_buffer.write_buffer_on (where); }
-
- void rewind () override
- { gdb_assert_not_reached ("mi_console_file::rewind called\n"); }
-
private:
/* The wrapped raw output stream. */
ui_file *m_raw;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 5e5289e..3c93553 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1542,7 +1542,7 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
}
else
{
- stream.rewind ();
+ stream.clear ();
print_scalar_formatted (&mbuf[col_byte], word_type, &opts,
word_asize, &stream);
uiout->field_stream (NULL, stream);
@@ -1553,7 +1553,7 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
{
int byte;
- stream.rewind ();
+ stream.clear ();
for (byte = row_byte;
byte < row_byte + word_size * nr_cols; byte++)
{
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 36db7a8..4b36edc 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -236,23 +236,31 @@ mi_ui_out::close (ui_out_type type)
m_suppress_field_separator = false;
}
+string_file *
+mi_ui_out::main_stream ()
+{
+ gdb_assert (m_streams.size () == 1);
+
+ return (string_file *) m_streams.back ();
+}
+
/* Clear the buffer. */
void
mi_ui_out::rewind ()
{
- ui_file_rewind (m_streams.back ());
+ main_stream ()->clear ();
}
/* Dump the buffer onto the specified stream. */
void
-mi_ui_out::put (ui_file *stream)
+mi_ui_out::put (ui_file *where)
{
- ui_file *outstream = m_streams.back ();
+ string_file *mi_stream = main_stream ();
- outstream->write_buffer_on (*stream);
- outstream->rewind ();
+ where->write (mi_stream->data (), mi_stream->size ());
+ mi_stream->clear ();
}
/* Return the current MI version. */
@@ -265,13 +273,12 @@ mi_ui_out::version ()
/* Constructor for an `mi_out_data' object. */
-mi_ui_out::mi_ui_out (int mi_version, string_file *stream)
+mi_ui_out::mi_ui_out (int mi_version)
: m_suppress_field_separator (false),
m_suppress_output (false),
m_mi_version (mi_version)
{
- gdb_assert (stream != NULL);
-
+ struct string_file *stream = new string_file ();
m_streams.push_back (stream);
}
@@ -284,9 +291,7 @@ mi_ui_out::~mi_ui_out ()
mi_ui_out *
mi_out_new (int mi_version)
{
- struct string_file *stream = new string_file ();
-
- return new mi_ui_out (mi_version, stream);
+ return new mi_ui_out (mi_version);
}
/* Helper function to return the given UIOUT as an mi_ui_out. It is an error
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index 26b6d72..fea94f2 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -30,7 +30,7 @@ class mi_ui_out : public ui_out
{
public:
- explicit mi_ui_out (int mi_version, string_file *stream);
+ explicit mi_ui_out (int mi_version);
virtual ~mi_ui_out ();
/* MI-specific */
@@ -78,6 +78,11 @@ private:
void open (const char *name, ui_out_type type);
void close (ui_out_type type);
+ /* Convenience method that returns the MI out's string stream cast
+ to its appropriate type. Assumes/asserts that output was not
+ redirected. */
+ string_file *main_stream ();
+
bool m_suppress_field_separator;
bool m_suppress_output;
int m_mi_version;
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 11fa83d..123a906 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -68,14 +68,14 @@ tui_disassemble (struct gdbarch *gdbarch, struct tui_asm_line *asm_lines,
asm_lines->addr = pc;
asm_lines->addr_string = xstrdup (gdb_dis_out.c_str ());
- gdb_dis_out.rewind ();
+ gdb_dis_out.clear ();
pc = pc + gdb_print_insn (gdbarch, pc, &gdb_dis_out, NULL);
asm_lines->insn = xstrdup (gdb_dis_out.c_str ());
/* Reset the buffer to empty. */
- gdb_dis_out.rewind ();
+ gdb_dis_out.clear ();
}
return pc;
}
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index ea94cd2..60e3274 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -102,12 +102,6 @@ ui_file_isatty (struct ui_file *file)
}
void
-ui_file_rewind (struct ui_file *file)
-{
- file->rewind ();
-}
-
-void
ui_file_write (struct ui_file *file,
const char *buf,
long length_buf)
@@ -146,18 +140,6 @@ string_file::write (const char *buf, long length_buf)
m_string.append (buf, length_buf);
}
-void
-string_file::write_buffer_on (ui_file &where)
-{
- where.write (m_string.data (), m_string.size ());
-}
-
-void
-string_file::rewind ()
-{
- m_string.clear ();
-}
-
stdio_file::stdio_file (FILE *file, bool close_p)
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index d3e0be2..3972d50 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -69,17 +69,6 @@ public:
virtual void flush ()
{}
-
- /* The following two methods are meant to be overridden by
- locally-buffered ui_files. */
-
- /* Rewind local buffer. I.e., clear it. */
- virtual void rewind ()
- {}
-
- /* Write contents of local buffer onto WHERE. */
- virtual void write_buffer_on (ui_file &where)
- {}
};
typedef std::unique_ptr<ui_file> ui_file_up;
@@ -99,8 +88,6 @@ extern null_file null_stream;
extern void gdb_flush (ui_file *);
-extern void ui_file_rewind (struct ui_file *stream);
-
extern int ui_file_isatty (struct ui_file *);
extern void ui_file_write (struct ui_file *file, const char *buf,
@@ -127,10 +114,6 @@ public:
long read (char *buf, long length_buf) override
{ gdb_assert_not_reached ("a string_file is not readable"); }
- void rewind () override;
-
- void write_buffer_on (ui_file &where) override;
-
/* string_file-specific public API. */
/* Accesses the std::string containing the entire output collected
@@ -144,6 +127,7 @@ public:
const char *c_str () const { return m_string.c_str (); }
size_t size () const { return m_string.size (); }
bool empty () const { return m_string.empty (); }
+ void clear () { return m_string.clear (); }
private:
/* The internal buffer. */
@@ -186,9 +170,6 @@ public:
bool isatty () override;
- void rewind () override
- { gdb_assert_not_reached ("can't rewind a stdio file"); }
-
private:
/* Sets the internal stream to FILE, and saves the FILE's file
descriptor in M_FD. */
@@ -262,9 +243,6 @@ public:
bool isatty () override;
void flush () override;
- void rewind () override
- { gdb_assert_not_reached ("tee_file::rewind called\n"); }
-
private:
/* The two underlying ui_files, and whether they should each be
closed on destruction. */
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 97535060..42cffbe 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -535,7 +535,7 @@ ui_out::field_stream (const char *fldname, string_file &stream)
field_string (fldname, stream.c_str ());
else
field_skip (fldname);
- stream.rewind ();
+ stream.clear ();
}
/* Used to omit a field. */
--
2.5.5