[PATCH] gdb: make use of RAII in run_inferior_call

[PATCH] gdb: make use of RAII in run_inferior_call

[Andrew Burgess]

In passing I noticed that there are three local variables in
run_inferior_call that are used to save, and then restore some state,
I think these could all be replaced with a RAII style scoped_restore
instead.

Of the three locals that I've changed, the only one that I believe is
now restored in a different location is ui::async, before this commit
the async field was restored after a call to either delete_file_handle
or ui_register_input_event_handler, and after this commit, the field
is restored before these calls.  However, I don't believe that either
of these functions depend on the value of the async field, so I
believe the commit is fine.

Tested on x86-64/Linux passes with no regressions.
---
 gdb/infcall.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 5365f97049c..9334648ac0e 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -578,21 +578,13 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
 		   struct thread_info *call_thread, CORE_ADDR real_pc)
 {
   struct gdb_exception caught_error;
-  int saved_in_infcall = call_thread->control.in_infcall;
   ptid_t call_thread_ptid = call_thread->ptid;
-  enum prompt_state saved_prompt_state = current_ui->prompt_state;
   int was_running = call_thread->state == THREAD_RUNNING;
-  int saved_ui_async = current_ui->async;
-
-  /* Infcalls run synchronously, in the foreground.  */
-  current_ui->prompt_state = PROMPT_BLOCKED;
-  /* So that we don't print the prompt prematurely in
-     fetch_inferior_event.  */
-  current_ui->async = 0;
 
   delete_file_handler (current_ui->input_fd);
 
-  call_thread->control.in_infcall = 1;
+  scoped_restore restore_in_infcall
+    = make_scoped_restore (&call_thread->control.in_infcall, 1);
 
   clear_proceed_status (0);
 
@@ -607,6 +599,15 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
 
   try
     {
+      /* Infcalls run synchronously, in the foreground.  */
+      scoped_restore restore_prompt_state
+	= make_scoped_restore (&current_ui->prompt_state, PROMPT_BLOCKED);
+
+      /* So that we don't print the prompt prematurely in
+	 fetch_inferior_event.  */
+      scoped_restore restore_ui_async
+	= make_scoped_restore (&current_ui->async, 0);
+
       proceed (real_pc, GDB_SIGNAL_0);
 
       /* Inferior function calls are always synchronous, even if the
@@ -622,12 +623,10 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
      so.  normal_stop calls async_enable_stdin, so reset the prompt
      state again here.  In other cases, stdin will be re-enabled by
      inferior_event_handler, when an exception is thrown.  */
-  current_ui->prompt_state = saved_prompt_state;
   if (current_ui->prompt_state == PROMPT_BLOCKED)
     delete_file_handler (current_ui->input_fd);
   else
     ui_register_input_event_handler (current_ui);
-  current_ui->async = saved_ui_async;
 
   /* If the infcall does NOT succeed, normal_stop will have already
      finished the thread states.  However, on success, normal_stop
@@ -663,8 +662,6 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
 	breakpoint_auto_delete (call_thread->control.stop_bpstat);
     }
 
-  call_thread->control.in_infcall = saved_in_infcall;
-
   return caught_error;
 }
 
-- 
2.25.4

Re: [PATCH] gdb: make use of RAII in run_inferior_call

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> In passing I noticed that there are three local variables in
> run_inferior_call that are used to save, and then restore some state,
> I think these could all be replaced with a RAII style scoped_restore
> instead.
>
> Of the three locals that I've changed, the only one that I believe is
> now restored in a different location is ui::async, before this commit
> the async field was restored after a call to either delete_file_handle
> or ui_register_input_event_handler, and after this commit, the field
> is restored before these calls.  However, I don't believe that either
> of these functions depend on the value of the async field, so I
> believe the commit is fine.

I've gone ahead and pushed this patch.

Thanks,
Andrew


>
> Tested on x86-64/Linux passes with no regressions.
> ---
>  gdb/infcall.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/infcall.c b/gdb/infcall.c
> index 5365f97049c..9334648ac0e 100644
> --- a/gdb/infcall.c
> +++ b/gdb/infcall.c
> @@ -578,21 +578,13 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
>  		   struct thread_info *call_thread, CORE_ADDR real_pc)
>  {
>    struct gdb_exception caught_error;
> -  int saved_in_infcall = call_thread->control.in_infcall;
>    ptid_t call_thread_ptid = call_thread->ptid;
> -  enum prompt_state saved_prompt_state = current_ui->prompt_state;
>    int was_running = call_thread->state == THREAD_RUNNING;
> -  int saved_ui_async = current_ui->async;
> -
> -  /* Infcalls run synchronously, in the foreground.  */
> -  current_ui->prompt_state = PROMPT_BLOCKED;
> -  /* So that we don't print the prompt prematurely in
> -     fetch_inferior_event.  */
> -  current_ui->async = 0;
>  
>    delete_file_handler (current_ui->input_fd);
>  
> -  call_thread->control.in_infcall = 1;
> +  scoped_restore restore_in_infcall
> +    = make_scoped_restore (&call_thread->control.in_infcall, 1);
>  
>    clear_proceed_status (0);
>  
> @@ -607,6 +599,15 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
>  
>    try
>      {
> +      /* Infcalls run synchronously, in the foreground.  */
> +      scoped_restore restore_prompt_state
> +	= make_scoped_restore (&current_ui->prompt_state, PROMPT_BLOCKED);
> +
> +      /* So that we don't print the prompt prematurely in
> +	 fetch_inferior_event.  */
> +      scoped_restore restore_ui_async
> +	= make_scoped_restore (&current_ui->async, 0);
> +
>        proceed (real_pc, GDB_SIGNAL_0);
>  
>        /* Inferior function calls are always synchronous, even if the
> @@ -622,12 +623,10 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
>       so.  normal_stop calls async_enable_stdin, so reset the prompt
>       state again here.  In other cases, stdin will be re-enabled by
>       inferior_event_handler, when an exception is thrown.  */
> -  current_ui->prompt_state = saved_prompt_state;
>    if (current_ui->prompt_state == PROMPT_BLOCKED)
>      delete_file_handler (current_ui->input_fd);
>    else
>      ui_register_input_event_handler (current_ui);
> -  current_ui->async = saved_ui_async;
>  
>    /* If the infcall does NOT succeed, normal_stop will have already
>       finished the thread states.  However, on success, normal_stop
> @@ -663,8 +662,6 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
>  	breakpoint_auto_delete (call_thread->control.stop_bpstat);
>      }
>  
> -  call_thread->control.in_infcall = saved_in_infcall;
> -
>    return caught_error;
>  }
>  
> -- 
> 2.25.4