Message ID | 20230511232203.247173-1-amerey@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 866ED3857734 for <patchwork@sourceware.org>; Thu, 11 May 2023 23:22:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 866ED3857734 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1683847359; bh=Nfw1IMZysjZAssPnEY5Osb4EaGq2H/wSPWBsPLnpD1E=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=AgyScZU1Cjsry7HH/YL7C7N1Lo4DKyUJy4/o64G7/I/vCmzM3boHGj2xK0/sv4Fyh 2oQuDLcjrIIJrTQWm4BR7PA7SjPsAfayCN3CqDuLJHZX+u2TarpLwWvoUpluAW1bmt /Oet3hNarURx51YwAAQCj+KRhE9jG/RE9mCFssHI= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 866183858CDA for <gdb-patches@sourceware.org>; Thu, 11 May 2023 23:22:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 866183858CDA Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-640-esz2OFSAOF-37g7sbvNg1w-1; Thu, 11 May 2023 19:22:14 -0400 X-MC-Unique: esz2OFSAOF-37g7sbvNg1w-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8F6E3185A78B for <gdb-patches@sourceware.org>; Thu, 11 May 2023 23:22:14 +0000 (UTC) Received: from localhost.localdomain (unknown [10.22.16.227]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5ED1A40C2076; Thu, 11 May 2023 23:22:14 +0000 (UTC) To: gdb-patches@sourceware.org Cc: Aaron Merey <amerey@redhat.com> Subject: [PATCH] gdb/cli-out.c: clear_current_line shouldn't trigger pagination prompt Date: Thu, 11 May 2023 19:22:03 -0400 Message-Id: <20230511232203.247173-1-amerey@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Aaron Merey <amerey@redhat.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
gdb/cli-out.c: clear_current_line shouldn't trigger pagination prompt
|
|
Commit Message
Aaron Merey
May 11, 2023, 11:22 p.m. UTC
clear_current_line overwrites the current line with chars_per_line blank spaces. Printing the final space triggers a condition in pager_file::puts that causes lines_printed to be incremented. If lines_printed becomes greater than or equal to lines_allowed, the pagination prompt will appear if enabled. In this case the prompt is unnecessary since after printing the final space clear_current_line immediately moves the cursor to the beginning of the line with '\r'. A new line isn't actually started, so the prompt ends up being spurious. Additionally it's possible for gdb to crash during this pagination prompt. Answering the prompt with 'q' throws an exception intended to bring gdb back to the main event loop. But since commit 0fea10f32746, clear_current_line may be called under the progress_update destructor. The exception will try to propagate through a destructor, causing an abort. This patch removes the possibility for clear_current_line to trigger the prompt by only printing chars_per_line - 1 blank spaces. clear_current_line is only ever called to clear download progress bars, which do not print anything on the final character of a line. An additional step we could take is to revert commit 0fea10f32746 to prevent any future possibility that an exception could be thrown during the progress_update destructor. --- gdb/cli-out.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
> Cc: Aaron Merey <amerey@redhat.com> > Date: Thu, 11 May 2023 19:22:03 -0400 > From: Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> > > This patch removes the possibility for clear_current_line to trigger the > prompt by only printing chars_per_line - 1 blank spaces. clear_current_line > is only ever called to clear download progress bars, which do not print > anything on the final character of a line. I'm not sure we should rely on the fact that the final character of the line is never there. clear_current_line is a general-purpose method, so it should do its job regardless. Can't we do something to handle the last character as well? E.g., could it be possible first to delete one character, so there are really only N-1 characters to fill with blank spaces? Thanks.
Hi Eli, On Fri, May 12, 2023 at 2:50 AM Eli Zaretskii <eliz@gnu.org> wrote: > > > Cc: Aaron Merey <amerey@redhat.com> > > Date: Thu, 11 May 2023 19:22:03 -0400 > > From: Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> > > > > This patch removes the possibility for clear_current_line to trigger the > > prompt by only printing chars_per_line - 1 blank spaces. clear_current_line > > is only ever called to clear download progress bars, which do not print > > anything on the final character of a line. > > I'm not sure we should rely on the fact that the final character of > the line is never there. clear_current_line is a general-purpose > method, so it should do its job regardless. > > Can't we do something to handle the last character as well? E.g., > could it be possible first to delete one character, so there are > really only N-1 characters to fill with blank spaces? We could rename clear_current_line to something like clear_progress_notify to help indicate that this is a special purpose function. We could also just disable pagination for the duration of clear_current_line. I'm not sure we can delete a character without incrementing chars_per_line. Aaron
> From: Aaron Merey <amerey@redhat.com> > Date: Mon, 15 May 2023 17:32:48 -0400 > Cc: gdb-patches@sourceware.org > > > I'm not sure we should rely on the fact that the final character of > > the line is never there. clear_current_line is a general-purpose > > method, so it should do its job regardless. > > > > Can't we do something to handle the last character as well? E.g., > > could it be possible first to delete one character, so there are > > really only N-1 characters to fill with blank spaces? > > We could rename clear_current_line to something like clear_progress_notify > to help indicate that this is a special purpose function. We could also > just disable pagination for the duration of clear_current_line. I think we should do both, thanks.
diff --git a/gdb/cli-out.c b/gdb/cli-out.c index 4c598883d4b..e93de1d0aa5 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -396,7 +396,7 @@ cli_ui_out::clear_current_line () chars_per_line = MAX_CHARS_PER_LINE; gdb_printf (stream, "\r"); - for (int i = 0; i < chars_per_line; ++i) + for (int i = 0; i < chars_per_line - 1; ++i) gdb_printf (stream, " "); gdb_printf (stream, "\r");