Re: [PATCH v2 1/3] gdb: Rename gdb_flush to ui_file_flush.

[Iain Buclaw <ibuclaw@gdcproject.org>]

Excerpts from Andrew Burgess's message of November 29, 2019 1:54 am:
> * Iain Buclaw <ibuclaw@gdcproject.org> [2019-11-28 23:53:48 +0000]:
> 
>> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
>> On Wednesday, 27 November 2019 00:00, Iain Buclaw <ibuclaw@gdcproject.org> wrote:
>>
>> > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
>> > On Tuesday, 26 November 2019 21:24, Pedro Alves palves@redhat.com wrote:
>> >
>> > > On 11/26/19 12:49 PM, Iain Buclaw wrote:
>> > >
>> > > > The significance of this is that printf_unfiltered writes messages to wrap_buffer, whereas puts_unfiltered pushes them immediately to stdout, resulting in "post-" messages being printed out of order.
>> > >
>> > > It sounds quite surprising that two _unfiltered functions could behave differently
>> > > like that. That sounds like a bug that should be fixed, instead of worked around
>> > > by having to recall to use printf vs puts.
>> > > Thanks,
>> > > Pedro Alves
>> >
>> > I think the best way to avoid the discrepancy is to treat both fputs_filtered and fputs_unfiltered equally by forwarding both calls to fputs_maybe_filtered.
>> >
>> > To avoid recursion, flush_wrap_buffer and fputs_maybe_filtered have had calls to fputs_unfiltered replaced with stream->puts().
>> >
>> > While attempting to grok my head around fputs_maybe_filtered, I also noticed that buffer_clearer is being removed by the compiler as dead code.
>> >
>>
>> Except that doesn't work as some parts of gdb require that direct
>> writing/flushing stdout is kept in place, so what I ultimately ended
>> up doing is defining two new functions for this, then repurposing
>> fputs_unfiltered and gdb_flush to interact with
>> fputs_maybe_unfiltered.
>>
>> I've split the patch into two logical parts, each dealing with one function at a time.  Running the testsuitelocally, I can see no new regressions as a result of applying this.
>>
>> This first patch renames gdb_flush to ui_file_flush.  Redefining gdb_flush in utils.c, with a new behaviour to flush the wrap_buffer if necessary before flushing the stream.
>>
>> --
>> Iain
>>
>> ---
>> gdb/ChangeLog:
>>
>> 2019-11-28  Iain Buclaw  <ibuclaw@gdcproject.org>
>>
>>         * gdb/event-loop.c (gdb_wait_for_event): Update.
>>         * gdb/printcmd.c (printf_command): Update.
>>         * gdb/remote-fileio.c (remote_fileio_func_write): Update.
>>         * gdb/remote-sim.c (gdb_os_flush_stdout): Update.
>>         (gdb_os_flush_stderr): Update.
>>         * gdb/remote.c (remote_console_output): Update.
>>         * gdb/ui-file.c (gdb_flush): Rename to...
>>         (ui_file_flush): ...this.
>>         (stderr_file::write): Update.
>>         (stderr_file::puts): Update.
>>         * gdb/ui-file.h (gdb_flush): Rename to...
>>         (ui_file_flush): ...this.
>>         * gdb/utils.c (gdb_flush): Add function.
>>         * gdb/utils.h (gdb_flush): Add declaration.
> 
> Iain,  thanks for looking at this issue.
> 
> When posting patches for review you should include the commit message
> for review too.  This should consist of a concise but informative
> title, a description of the problem, maybe how you discovered it, or
> what problems it was causing, and then an overview of the solution.
> 

Hi Andrew,

Sorry, I've been awfully held up with matters out of my own hands, and
eventually enough time passed that this message fell through the cracks.

Refamiliarizing myself with what I wrote, I've noticed that its already
been committed.  Though I don't think it was I who pushed it.

http://sourceware.org/git/?p=binutils-gdb.git;a=log;h=899016d49d289757372459f72d642a4c6b3b7732

Is there anything left for me to address here?

Iain.