gdb/cli-out.c: clear_current_line shouldn't trigger pagination prompt

  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(-)
  

> 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.