From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [IPv6:2001:67c:2050::465:102]) by sourceware.org (Postfix) with ESMTPS id 2928D3857C4E for ; Fri, 7 Aug 2020 21:01:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2928D3857C4E Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:105:465:1:1:0]) (using TLSv1.2 with cipher ECDHE-RSA-CHACHA20-POLY1305 (256/256 bits)) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4BNd886llJzKmDK; Fri, 7 Aug 2020 23:01:44 +0200 (CEST) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by hefe.heinlein-support.de (hefe.heinlein-support.de [91.198.250.172]) (amavisd-new, port 10030) with ESMTP id vokK3Wq66oBL; Fri, 7 Aug 2020 23:01:40 +0200 (CEST) Date: Fri, 07 Aug 2020 23:01:37 +0200 From: Iain Buclaw Subject: Re: [PATCH v2 1/3] gdb: Rename gdb_flush to ui_file_flush. To: Andrew Burgess Cc: "gdb-patches@sourceware.org" , Pedro Alves References: <20191129005420.GF3410@embecosm.com> In-Reply-To: <20191129005420.GF3410@embecosm.com> MIME-Version: 1.0 Message-Id: <1596833341.t1ehhpwvpi.astroid@galago.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-MBO-SPAM-Probability: X-Rspamd-Score: -3.91 / 15.00 / 15.00 X-Rspamd-Queue-Id: 9FC46181D X-Rspamd-UID: 1d6f5a X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Aug 2020 21:01:48 -0000 Excerpts from Andrew Burgess's message of November 29, 2019 1:54 am: > * Iain Buclaw [2019-11-28 23:53:48 +0000]: >=20 >> =E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90 Original= Message =E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90 >> On Wednesday, 27 November 2019 00:00, Iain Buclaw wrote: >> >> > =E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90 Origin= al Message =E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90 >> > On Tuesday, 26 November 2019 21:24, Pedro Alves palves@redhat.com wrot= e: >> > >> > > 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 beha= ve differently >> > > like that. That sounds like a bug that should be fixed, instead of w= orked 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_f= iltered and fputs_unfiltered equally by forwarding both calls to fputs_mayb= e_filtered. >> > >> > To avoid recursion, flush_wrap_buffer and fputs_maybe_filtered have ha= d calls to fputs_unfiltered replaced with stream->puts(). >> > >> > While attempting to grok my head around fputs_maybe_filtered, I also n= oticed 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 funct= ion 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_flu= sh in utils.c, with a new behaviour to flush the wrap_buffer if necessary b= efore flushing the stream. >> >> -- >> Iain >> >> --- >> gdb/ChangeLog: >> >> 2019-11-28 Iain Buclaw >> >> * 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. >=20 > Iain, thanks for looking at this issue. >=20 > 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. >=20 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=3Dbinutils-gdb.git;a=3Dlog;h=3D899016d49d28975= 7372459f72d642a4c6b3b7732 Is there anything left for me to address here? Iain.