This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 1/2] Remove cleanups from gdb_readline_wrapper
Hi Tom,
On 03/23/2018 03:38 AM, Tom Tromey wrote:
> char *
> gdb_readline_wrapper (const char *prompt)
> {
> struct ui *ui = current_ui;
> - struct cleanup *back_to;
> - struct gdb_readline_wrapper_cleanup *cleanup;
> - char *retval;
> -
> - cleanup = XNEW (struct gdb_readline_wrapper_cleanup);
> - cleanup->handler_orig = ui->input_handler;
> - ui->input_handler = gdb_readline_wrapper_line;
> -
> - if (ui->command_editing)
> - cleanup->already_prompted_orig = rl_already_prompted;
> - else
> - cleanup->already_prompted_orig = 0;
> -
> - cleanup->target_is_async_orig = target_is_async_p ();
>
> - ui->secondary_prompt_depth++;
> - back_to = make_cleanup (gdb_readline_wrapper_cleanup, cleanup);
> + gdb_readline_wrapper_cleanup cleanup (ui);
>
> /* Processing events may change the current UI. */
> scoped_restore save_ui = make_scoped_restore (¤t_ui);
>
> - if (cleanup->target_is_async_orig)
> + if (cleanup.target_is_async_orig)
> target_async (0);
I think we can move these to the ctor too, and then all the fields
of the structure can be made private. Something like the patch
below. (Tested on x86-64 GNU/Linux.)
While at it, since we're touching most of the structure, we might as
well reindent it to have open { at column 0. (Not done below to keep
the patch readable.)
WDYT?
>From a98b9bc98522351f757e3835b22bdfa9ecf2f997 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sat, 24 Mar 2018 10:55:31 +0000
Subject: [PATCH] gdb_readline_wrapper_cleanup
---
gdb/top.c | 50 +++++++++++++++++++++++++++-----------------------
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/gdb/top.c b/gdb/top.c
index 02d23cca667..523530ffbae 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -917,15 +917,21 @@ gdb_readline_wrapper_line (char *line)
gdb_rl_callback_handler_remove ();
}
-struct gdb_readline_wrapper_cleanup
+class gdb_readline_wrapper_cleanup
{
- explicit gdb_readline_wrapper_cleanup (struct ui *ui)
- : handler_orig (ui->input_handler),
- already_prompted_orig (ui->command_editing ? rl_already_prompted : 0),
- target_is_async_orig (target_is_async_p ())
+ public:
+ explicit gdb_readline_wrapper_cleanup ()
+ : m_handler_orig (current_ui->input_handler),
+ m_already_prompted_orig (current_ui->command_editing
+ ? rl_already_prompted : 0),
+ m_target_is_async_orig (target_is_async_p ()),
+ m_save_ui (¤t_ui)
{
- ui->input_handler = gdb_readline_wrapper_line;
- ui->secondary_prompt_depth++;
+ current_ui->input_handler = gdb_readline_wrapper_line;
+ current_ui->secondary_prompt_depth++;
+
+ if (m_target_is_async_orig)
+ target_async (0);
}
~gdb_readline_wrapper_cleanup ()
@@ -933,10 +939,10 @@ struct gdb_readline_wrapper_cleanup
struct ui *ui = current_ui;
if (ui->command_editing)
- rl_already_prompted = already_prompted_orig;
+ rl_already_prompted = m_already_prompted_orig;
gdb_assert (ui->input_handler == gdb_readline_wrapper_line);
- ui->input_handler = handler_orig;
+ ui->input_handler = m_handler_orig;
/* Don't restore our input handler in readline yet. That would make
readline prep the terminal (putting it in raw mode), while the
@@ -953,38 +959,36 @@ struct gdb_readline_wrapper_cleanup
after_char_processing_hook = saved_after_char_processing_hook;
saved_after_char_processing_hook = NULL;
- if (target_is_async_orig)
+ if (m_target_is_async_orig)
target_async (1);
}
DISABLE_COPY_AND_ASSIGN (gdb_readline_wrapper_cleanup);
- void (*handler_orig) (char *);
- int already_prompted_orig;
+ private:
+ void (*m_handler_orig) (char *);
+ int m_already_prompted_orig;
/* Whether the target was async. */
- int target_is_async_orig;
+ bool m_target_is_async_orig;
+
+ /* Processing events may change the current UI. */
+ scoped_restore_tmpl<struct ui *> m_save_ui;
};
char *
gdb_readline_wrapper (const char *prompt)
{
- struct ui *ui = current_ui;
-
- gdb_readline_wrapper_cleanup cleanup (ui);
-
- /* Processing events may change the current UI. */
- scoped_restore save_ui = make_scoped_restore (¤t_ui);
-
- if (cleanup.target_is_async_orig)
- target_async (0);
+ /* This saves/restores global readline/gdb state around event
+ processing. */
+ gdb_readline_wrapper_cleanup cleanup;
/* Display our prompt and prevent double prompt display. Don't pass
down a NULL prompt, since that has special meaning for
display_gdb_prompt -- it indicates a request to print the primary
prompt, while we want a secondary prompt here. */
display_gdb_prompt (prompt != NULL ? prompt : "");
- if (ui->command_editing)
+ if (current_ui->command_editing)
rl_already_prompted = 1;
if (after_char_processing_hook)
--
2.14.3