* [RFA 0/2] two simple cleanup removals @ 2018-03-23 3:38 Tom Tromey 2018-03-23 3:38 ` [RFA 1/2] Remove cleanups from gdb_readline_wrapper Tom Tromey 2018-03-23 3:38 ` [RFA 2/2] Remove cleanups from prompt_for_continue Tom Tromey 0 siblings, 2 replies; 6+ messages in thread From: Tom Tromey @ 2018-03-23 3:38 UTC (permalink / raw) To: gdb-patches Here are a couple of simple cleanup removal patches. These were regression tested by the buildbot. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFA 1/2] Remove cleanups from gdb_readline_wrapper 2018-03-23 3:38 [RFA 0/2] two simple cleanup removals Tom Tromey @ 2018-03-23 3:38 ` Tom Tromey 2018-03-24 11:19 ` Pedro Alves 2018-03-23 3:38 ` [RFA 2/2] Remove cleanups from prompt_for_continue Tom Tromey 1 sibling, 1 reply; 6+ messages in thread From: Tom Tromey @ 2018-03-23 3:38 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This removes some cleanups from gdb_readline_wrapper by changing the existing gdb_readline_wrapper_cleanup struct to have a constructor and destructor, and then changing gdb_readline_wrapper to simply instantiate it on the stack. gdb/ChangeLog 2018-03-22 Tom Tromey <tom@tromey.com> * top.c (struct gdb_readline_wrapper_cleanup): Add constructor, destructor. (gdb_readline_wrapper_cleanup): Remove function. (gdb_readline_wrapper): Remove cleanups. --- gdb/ChangeLog | 7 +++++ gdb/top.c | 91 ++++++++++++++++++++++++++--------------------------------- 2 files changed, 47 insertions(+), 51 deletions(-) diff --git a/gdb/top.c b/gdb/top.c index 14387b9f1a..02d23cca66 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -919,73 +919,64 @@ gdb_readline_wrapper_line (char *line) struct gdb_readline_wrapper_cleanup { - void (*handler_orig) (char *); - int already_prompted_orig; + 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 ()) + { + ui->input_handler = gdb_readline_wrapper_line; + ui->secondary_prompt_depth++; + } - /* Whether the target was async. */ - int target_is_async_orig; - }; + ~gdb_readline_wrapper_cleanup () + { + struct ui *ui = current_ui; -static void -gdb_readline_wrapper_cleanup (void *arg) -{ - struct ui *ui = current_ui; - struct gdb_readline_wrapper_cleanup *cleanup - = (struct gdb_readline_wrapper_cleanup *) arg; + if (ui->command_editing) + rl_already_prompted = already_prompted_orig; - if (ui->command_editing) - rl_already_prompted = cleanup->already_prompted_orig; + gdb_assert (ui->input_handler == gdb_readline_wrapper_line); + ui->input_handler = handler_orig; - gdb_assert (ui->input_handler == gdb_readline_wrapper_line); - ui->input_handler = cleanup->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 + line we just read may trigger execution of a command that expects + the terminal in the default cooked/canonical mode, such as e.g., + running Python's interactive online help utility. See + gdb_readline_wrapper_line for when we'll reinstall it. */ - /* Don't restore our input handler in readline yet. That would make - readline prep the terminal (putting it in raw mode), while the - line we just read may trigger execution of a command that expects - the terminal in the default cooked/canonical mode, such as e.g., - running Python's interactive online help utility. See - gdb_readline_wrapper_line for when we'll reinstall it. */ + gdb_readline_wrapper_result = NULL; + gdb_readline_wrapper_done = 0; + ui->secondary_prompt_depth--; + gdb_assert (ui->secondary_prompt_depth >= 0); - gdb_readline_wrapper_result = NULL; - gdb_readline_wrapper_done = 0; - ui->secondary_prompt_depth--; - gdb_assert (ui->secondary_prompt_depth >= 0); + after_char_processing_hook = saved_after_char_processing_hook; + saved_after_char_processing_hook = NULL; - after_char_processing_hook = saved_after_char_processing_hook; - saved_after_char_processing_hook = NULL; + if (target_is_async_orig) + target_async (1); + } - if (cleanup->target_is_async_orig) - target_async (1); + DISABLE_COPY_AND_ASSIGN (gdb_readline_wrapper_cleanup); - xfree (cleanup); -} + void (*handler_orig) (char *); + int already_prompted_orig; + + /* Whether the target was async. */ + int target_is_async_orig; + }; 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); /* Display our prompt and prevent double prompt display. Don't pass @@ -1004,9 +995,7 @@ gdb_readline_wrapper (const char *prompt) if (gdb_readline_wrapper_done) break; - retval = gdb_readline_wrapper_result; - do_cleanups (back_to); - return retval; + return gdb_readline_wrapper_result; } \f -- 2.13.6 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 1/2] Remove cleanups from gdb_readline_wrapper 2018-03-23 3:38 ` [RFA 1/2] Remove cleanups from gdb_readline_wrapper Tom Tromey @ 2018-03-24 11:19 ` Pedro Alves 2018-03-25 16:19 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Pedro Alves @ 2018-03-24 11:19 UTC (permalink / raw) To: Tom Tromey, gdb-patches 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 1/2] Remove cleanups from gdb_readline_wrapper 2018-03-24 11:19 ` Pedro Alves @ 2018-03-25 16:19 ` Tom Tromey 0 siblings, 0 replies; 6+ messages in thread From: Tom Tromey @ 2018-03-25 16:19 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> I think we can move these to the ctor too, and then all the fields Pedro> of the structure can be made private. Something like the patch Pedro> below. (Tested on x86-64 GNU/Linux.) Seems like a good idea to me. Pedro> While at it, since we're touching most of the structure, we might as Pedro> well reindent it to have open { at column 0. (Not done below to keep Pedro> the patch readable.) I added this and did the reindentation. Also I removed the "explicit" from the constructor. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFA 2/2] Remove cleanups from prompt_for_continue 2018-03-23 3:38 [RFA 0/2] two simple cleanup removals Tom Tromey 2018-03-23 3:38 ` [RFA 1/2] Remove cleanups from gdb_readline_wrapper Tom Tromey @ 2018-03-23 3:38 ` Tom Tromey 2018-03-24 11:19 ` Pedro Alves 1 sibling, 1 reply; 6+ messages in thread From: Tom Tromey @ 2018-03-23 3:38 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This removes the cleanups from prompt_for_continue by the use of unique_xmalloc_ptr. gdb/ChangeLog 2018-03-22 Tom Tromey <tom@tromey.com> * utils.c (prompt_for_continue): Use unique_xmalloc_ptr. --- gdb/ChangeLog | 4 ++++ gdb/utils.c | 9 ++------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/gdb/utils.c b/gdb/utils.c index 3886efd840..ee31f39661 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1479,9 +1479,7 @@ set_screen_width_and_height (int width, int height) static void prompt_for_continue (void) { - char *ignore; char cont_prompt[120]; - struct cleanup *old_chain = make_cleanup (null_cleanup, NULL); /* Used to add duration we waited for user to respond to prompt_for_continue_wait_time. */ using namespace std::chrono; @@ -1504,8 +1502,7 @@ prompt_for_continue (void) /* Call gdb_readline_wrapper, not readline, in order to keep an event loop running. */ - ignore = gdb_readline_wrapper (cont_prompt); - make_cleanup (xfree, ignore); + gdb::unique_xmalloc_ptr<char> ignore (gdb_readline_wrapper (cont_prompt)); /* Add time spend in this routine to prompt_for_continue_wait_time. */ prompt_for_continue_wait_time += steady_clock::now () - prompt_started; @@ -1515,7 +1512,7 @@ prompt_for_continue (void) if (ignore != NULL) { - char *p = ignore; + char *p = ignore.get (); while (*p == ' ' || *p == '\t') ++p; @@ -1529,8 +1526,6 @@ prompt_for_continue (void) reinitialize_more_filter (); dont_repeat (); /* Forget prev cmd -- CR won't repeat it. */ - - do_cleanups (old_chain); } /* Initialize timer to keep track of how long we waited for the user. */ -- 2.13.6 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 2/2] Remove cleanups from prompt_for_continue 2018-03-23 3:38 ` [RFA 2/2] Remove cleanups from prompt_for_continue Tom Tromey @ 2018-03-24 11:19 ` Pedro Alves 0 siblings, 0 replies; 6+ messages in thread From: Pedro Alves @ 2018-03-24 11:19 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 03/23/2018 03:38 AM, Tom Tromey wrote: > This removes the cleanups from prompt_for_continue by the use of > unique_xmalloc_ptr. > > gdb/ChangeLog > 2018-03-22 Tom Tromey <tom@tromey.com> > > * utils.c (prompt_for_continue): Use unique_xmalloc_ptr. OK. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-25 16:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-23 3:38 [RFA 0/2] two simple cleanup removals Tom Tromey 2018-03-23 3:38 ` [RFA 1/2] Remove cleanups from gdb_readline_wrapper Tom Tromey 2018-03-24 11:19 ` Pedro Alves 2018-03-25 16:19 ` Tom Tromey 2018-03-23 3:38 ` [RFA 2/2] Remove cleanups from prompt_for_continue Tom Tromey 2018-03-24 11:19 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).