Re: [RFA] [1/4] Remove libgdb API (gdb_breakpoint_query)

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi Keith,

On Fri, 13 Jan 2012 20:59:10 +0100, Keith Seitz wrote:
> This is the first patch in a series of four which aims to remove the
> libgdb API (aka gdb.h) from gdb.

looks great.


> At the same time, it also necessitated/facilitated a related cleanup
> of various breakpoint.c functions which did something like:
> 
> void
> foo (blah, blah, blah)
> {
>    struct ui_out *uiout = current_uiout;
>    /* ... */
> 
> instead of simply passing in uiout as an argument.

This is sure a great idea but it is done only a half way.

Former wrapper catch_exceptions_with_msg replaces CURRENT_UIOUT temporarily,
therefore it applies even to all the callees not patched in this patchset.

With this change those callees access unchanged CURRENT_UIOUT which is
a regression.

Maybe could you more thoroughly review all the uses of CURRENT_UIOUT?

Used this patch on top of your whole patchset and it causes regressions for:
	gdb.ada/mi_catch_ex.exp gdb.mi/mi-nonstop.exp gdb.mi/mi-nsmoribund.exp
	gdb.mi/mi-pthreads.exp gdb.mi/mi2-pthreads.exp
	gdb.python/py-evthreads.exp gdb.threads/hand-call-in-threads.exp
	gdb.threads/linux-dp.exp gdb.threads/no-unwaited-for-left.exp
	gdb.threads/tls.exp

Such as:

Program terminated with signal 11, Segmentation fault.
#0  0x00000000007bc12f in ui_out_is_mi_like_p (uiout=0x0) at ui-out.c:736
736	  return uiout->impl->is_mi_like_p;
(gdb) up
#1  0x00000000006fa93d in print_stack_frame (frame=0x41cfb80, print_level=1, print_what=SRC_AND_LOC) at stack.c:158
158	  if (ui_out_is_mi_like_p (current_uiout))
(gdb) bt
#0  in ui_out_is_mi_like_p (uiout=0x0) at ui-out.c:736
#1  in print_stack_frame (frame=0x41cfb80, print_level=1, print_what=SRC_AND_LOC) at stack.c:158
#2  in thread_select (uiout=0x3c47bd0, tidstr=0x407faa0 "5") at thread.c:1403
#3  in mi_cmd_thread_select (command=0x4231030 "thread-select", argv=0x41af000, argc=1) at ./mi/mi-main.c:470
#4  in mi_cmd_execute (parse=0x422c3c0) at ./mi/mi-main.c:2090
#5  in captured_mi_execute_command (uiout=0x3c47bd0, context=0x422c3c0) at ./mi/mi-main.c:1834
#6  in mi_execute_command (cmd=0x41d4f40 "-thread-select 5", from_tty=1) at ./mi/mi-main.c:1956
#7  in mi_execute_command_wrapper (cmd=0x41d4f40 "-thread-select 5") at ./mi/mi-interp.c:290
#8  in gdb_readline2 (client_data=0x0) at event-top.c:717
#9  in stdin_event_handler (error=0, client_data=0x0) at event-top.c:375
#10 in handle_file_event (data=...) at event-loop.c:827


Thanks,
Jan


--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5037,17 +5037,30 @@ print_one_breakpoint (struct ui_out *uiout,
     }
 }
 
+void
+restore_current_uiout (void *arg)
+{
+  gdb_assert (current_uiout == NULL);
+  current_uiout = arg;
+}
+
 /* Print information about breakpoint B to UIOUT.  */
 
 void
 breakpoint_query (struct ui_out *uiout, struct breakpoint *b)
 {
+extern void restore_current_uiout (void *arg);
+  struct cleanup *back_to = make_cleanup (restore_current_uiout, current_uiout);
+  current_uiout = NULL;
+
   if (b != NULL)
     {
       struct bp_location *dummy_loc = NULL;
 
       print_one_breakpoint (uiout, b, &dummy_loc, 0);
     }
+
+  do_cleanups (back_to);
 }
 
 static int
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -486,6 +486,10 @@ list_thread_ids (struct ui_out *uiout)
   struct cleanup *cleanup_chain;
   int current_thread = -1;
 
+extern void restore_current_uiout (void *arg);
+  struct cleanup *back_to = make_cleanup (restore_current_uiout, current_uiout);
+  current_uiout = NULL;
+
   update_thread_list ();
 
   cleanup_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "thread-ids");
@@ -507,6 +511,8 @@ list_thread_ids (struct ui_out *uiout)
   if (current_thread != -1)
     ui_out_field_int (uiout, "current-thread-id", current_thread);
   ui_out_field_int (uiout, "number-of-threads", num);
+
+  do_cleanups (back_to);
 }
 
 /* Return true if TP is an active thread.  */
@@ -1363,6 +1369,10 @@ thread_select (struct ui_out *uiout, char *tidstr)
   int num;
   struct thread_info *tp;
 
+extern void restore_current_uiout (void *arg);
+  struct cleanup *back_to = make_cleanup (restore_current_uiout, current_uiout);
+  current_uiout = NULL;
+
   num = value_as_long (parse_and_eval (tidstr));
 
   tp = find_thread_id (num);
@@ -1396,6 +1406,8 @@ thread_select (struct ui_out *uiout, char *tidstr)
   /* Since the current thread may have changed, see if there is any
      exited thread we can now delete.  */
   prune_threads ();
+
+  do_cleanups (back_to);
 }
 
 void