[master+7.12,v2,1/3] Introduce cleanup to restore current_uiout

Message ID 20160914174548.5873-2-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Sept. 14, 2016, 5:45 p.m. UTC
  Make a globally available cleanup from a pre-existing one in infrun.c.
This is used in the following patch.

gdb/ChangeLog:

	* infrun.c (restore_current_uiout_cleanup): Move to ui-out.c.
	(print_stop_event): Use make_cleanup_restore_current_uiout.
	* ui-out.c (restore_current_uiout_cleanup): Move from infrun.c.
	(make_cleanup_restore_current_uiout): New function definition.
	* ui-out.h (make_cleanup_restore_current_uiout): New function
	definition.
---
 gdb/infrun.c | 12 +-----------
 gdb/ui-out.c | 18 ++++++++++++++++++
 gdb/ui-out.h |  4 ++++
 3 files changed, 23 insertions(+), 11 deletions(-)
  

Comments

Pedro Alves Sept. 14, 2016, 5:56 p.m. UTC | #1
On 09/14/2016 06:45 PM, Simon Marchi wrote:
> Make a globally available cleanup from a pre-existing one in infrun.c.
> This is used in the following patch.
> 
> gdb/ChangeLog:
> 
> 	* infrun.c (restore_current_uiout_cleanup): Move to ui-out.c.
> 	(print_stop_event): Use make_cleanup_restore_current_uiout.
> 	* ui-out.c (restore_current_uiout_cleanup): Move from infrun.c.
> 	(make_cleanup_restore_current_uiout): New function definition.
> 	* ui-out.h (make_cleanup_restore_current_uiout): New function
> 	definition.

OK.

Thanks,
Pedro Alves
  
Tom Tromey Sept. 14, 2016, 6:10 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> -  old_chain = make_cleanup (restore_current_uiout_cleanup, current_uiout);
Simon> +  old_chain = make_cleanup_restore_current_uiout ();

There's also already make_cleanup_restore_ui_out.
You could just use "make_cleanup_restore_ui_out (&current_uiout)" instead.

Tom
  
Simon Marchi Sept. 14, 2016, 6:18 p.m. UTC | #3
On 16-09-14 02:10 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> -  old_chain = make_cleanup (restore_current_uiout_cleanup, current_uiout);
> Simon> +  old_chain = make_cleanup_restore_current_uiout ();
> 
> There's also already make_cleanup_restore_ui_out.
> You could just use "make_cleanup_restore_ui_out (&current_uiout)" instead.
> 
> Tom

Ah good point, I'll probably do that.

Thanks!
  
Pedro Alves Sept. 14, 2016, 6:32 p.m. UTC | #4
On 09/14/2016 07:18 PM, Simon Marchi wrote:
> On 16-09-14 02:10 PM, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>>
>> Simon> -  old_chain = make_cleanup (restore_current_uiout_cleanup, current_uiout);
>> Simon> +  old_chain = make_cleanup_restore_current_uiout ();
>>
>> There's also already make_cleanup_restore_ui_out.
>> You could just use "make_cleanup_restore_ui_out (&current_uiout)" instead.
>>
>> Tom
> 
> Ah good point, I'll probably do that.

FWIW, I like the newer spelling better since we'll always be
restoring current_uiout, but it's not a big deal at all.
(similarly to https://sourceware.org/ml/gdb-patches/2016-09/msg00060.html)

Thanks,
Pedro Alves
  
Tom Tromey Sept. 14, 2016, 7:12 p.m. UTC | #5
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> FWIW, I like the newer spelling better since we'll always be
Pedro> restoring current_uiout, but it's not a big deal at all.

The only existing call to make_cleanup_restore_uiout uses current_uiout,
so another option would be to just replace that.

Tom
  
Simon Marchi Sept. 15, 2016, 3:17 a.m. UTC | #6
On 2016-09-14 15:12, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> FWIW, I like the newer spelling better since we'll always be
> Pedro> restoring current_uiout, but it's not a big deal at all.
> 
> The only existing call to make_cleanup_restore_uiout uses 
> current_uiout,
> so another option would be to just replace that.
> 
> Tom

Ah then it makes sense to use make_cleanup_restore_current_uiout.  Why 
have a parametrized version when we don't need it?  I'll post an updated 
patch.
  

Patch

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 70d7a09..ec37ca1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8096,16 +8096,6 @@  print_stop_location (struct target_waitstatus *ws)
     print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
 }
 
-/* Cleanup that restores a previous current uiout.  */
-
-static void
-restore_current_uiout_cleanup (void *arg)
-{
-  struct ui_out *saved_uiout = (struct ui_out *) arg;
-
-  current_uiout = saved_uiout;
-}
-
 /* See infrun.h.  */
 
 void
@@ -8118,7 +8108,7 @@  print_stop_event (struct ui_out *uiout)
 
   get_last_target_status (&last_ptid, &last);
 
-  old_chain = make_cleanup (restore_current_uiout_cleanup, current_uiout);
+  old_chain = make_cleanup_restore_current_uiout ();
   current_uiout = uiout;
 
   print_stop_location (&last);
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 3972a56..ec44ab6 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -953,6 +953,24 @@  ui_out_destroy (struct ui_out *uiout)
   xfree (uiout);
 }
 
+/* Cleanup that restores a previous current uiout.  */
+
+static void
+restore_current_uiout_cleanup (void *arg)
+{
+  struct ui_out *saved_uiout = (struct ui_out *) arg;
+
+  current_uiout = saved_uiout;
+}
+
+/* See ui-out.h.  */
+
+struct cleanup *
+make_cleanup_restore_current_uiout (void)
+{
+  return make_cleanup (restore_current_uiout_cleanup, current_uiout);
+}
+
 /* Standard gdb initialization hook.  */
 
 void
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 9e1e74d..6a4d78a 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -247,4 +247,8 @@  extern void ui_out_destroy (struct ui_out *uiout);
 
 extern int ui_out_redirect (struct ui_out *uiout, struct ui_file *outstream);
 
+/* Make a cleanup that restores the previous current uiout.  */
+
+extern struct cleanup *make_cleanup_restore_current_uiout (void);
+
 #endif /* UI_OUT_H */