From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout-p-101.mailbox.org (mout-p-101.mailbox.org [80.241.56.151]) by sourceware.org (Postfix) with ESMTPS id 37E8838930FC for ; Fri, 7 Aug 2020 22:15:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 37E8838930FC 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-101.mailbox.org (Postfix) with ESMTPS id 4BNfnM5zftzKmsd; Sat, 8 Aug 2020 00:15:35 +0200 (CEST) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by spamfilter01.heinlein-hosting.de (spamfilter01.heinlein-hosting.de [80.241.56.115]) (amavisd-new, port 10030) with ESMTP id x1jYUFLSComQ; Sat, 8 Aug 2020 00:15:32 +0200 (CEST) Date: Sat, 08 Aug 2020 00:15:29 +0200 From: Iain Buclaw Subject: Re: [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts. To: Joel Brobecker Cc: "gdb-patches@sourceware.org" , Pedro Alves References: <20200126113139.GA21996@adacore.com> In-Reply-To: <20200126113139.GA21996@adacore.com> MIME-Version: 1.0 Message-Id: <1596836831.4cu2ynah7y.astroid@galago.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-MBO-SPAM-Probability: X-Rspamd-Score: -1.79 / 15.00 / 15.00 X-Rspamd-Queue-Id: A7870182A X-Rspamd-UID: d2b9de X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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 22:15:39 -0000 Excerpts from Joel Brobecker's message of January 26, 2020 12:31 pm: > Hi Iain, >=20 >> This patch redefines fputs_unfiltered in utils.c, with new behavior to f= orward parameters to fputs_maybe_filtered. This makes fputs_unfiltered ide= ntical to fputs_filtered, except filtering is disabled. >> >> Some callers of fputs_unfiltered have been updated to use ui_file_puts w= here they were using other ui_file_* functions anyway for IO. >> >> This fixes the problem I saw with \032\032post-prompt annotation being f= lushed to stdout in the wrong order. >> >> -- >> Iain >> >> --- >> gdb/ChangeLog: >> >> 2019-11-29 Iain Buclaw >> >> * gdb/remote-sim.c (gdb_os_write_stderr): Update. >> * gdb/remote.c (remote_console_output): Update. >> * gdb/ui-file.c (fputs_unfiltered): Rename to... >> (ui_file_puts): ...this. >> * gdb/ui-file.h (ui_file_puts): Add declaration. >> * gdb/utils.c (emit_style_escape): Update. >> (flush_wrap_buffer): Update. >> (fputs_maybe_filtered): Update. >> (fputs_unfiltered): Add function. >=20 > As far as I can tell, no one has reviewed this patch. This is a call > for help from other Global Maintainers, as I don't feel qualified > to review it all on my own. >=20 > As far as I can tell from reading this patch, it really feels like > you are trying to turn the turn the relationship of filtered vs > unfiltered on its head. Right now, fputs_maybe_filtered is the one > that depends on fputs_unfiltered, and this is realy what feels > natural to me. >=20 > For some reason, you decided to do the other way around, but without > a real explanation of why we are getting text being printed in the wrong > order, and why your patch fixes it or why you think this is the best > approach. >=20 >=20 Hi Joel, I guess that someone answered your question, as it appears that this patch got committed anyway. To give my best explanation on what I saw anyway. There are a number of print related functions that each followed a consistent behaviour except fputs_unfiltered. - printf_filtered called vfprintf_maybe_filtered(/*filter=3D*/true). - printf_unfiltered called vfprintf_maybe_filtered(false). - fprintf_filtered called vfprintf_maybe_filtered(true). - fprintf_unfiltered called vfprintf_maybe_filtered(false). - fputs_styled called fputs_maybe_filtered(/*filter=3D*/1). - fputs_styled_unfiltered called fputs_maybe_filtered(0). - fputs_filtered called fputs_maybe_filtered(1). - fputs_unfiltered called ui_file::puts() Before 2a3c1174c3c0db1140180fb3fc56ac324d1c0a7c, all *printf_unfiltered functions used to call fputs_unfiltered(), so there was no problems mixing printf_unfiltered() and fputs_unfiltered(). After the change though, printf_unfiltered() now got written to a buffer awaiting to be flushed whilst fputs_unfiltered() continued to write directly to the output file. This is the part that caused the regression: --- @@ -2064,13 +2096,13 @@ vfprintf_unfiltered (struct ui_file *stream, const = char *format, va_list args) fputs_unfiltered (timestamp.c_str (), stream); } else - fputs_unfiltered (linebuffer.c_str (), stream); + vfprintf_maybe_filtered (stream, format, args, false, true); } void vprintf_filtered (const char *format, va_list args) --- Reverting the one line caused other problems however, so the only way I saw to reasonably move forward was to bring fputs_unfiltered in line with all others I've listed above, so now it calls fputs_maybe_filtered(0), and no one has to worry about printf_unfiltered and fputs_unfiltered being flushed in a different order to the one they were called in. Hope this is clear. Iain.