Re: [RFA] bpstat_do_actions in one place

[Vladimir Prus <vladimir@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

On Friday 02 May 2008 06:57:53 Daniel Jacobowitz wrote:
> On Thu, Apr 24, 2008 at 07:02:03PM +0300, Vladimir Prus wrote:
> > 
> > This patch, to be applied on top of the "Use observers to report stop events."
> > make async code call bpstat_do_action in the event handler, to make sure that
> > it's always called.
> > 
> > OK?
> 
> OK.
> 
> > +      /* If there's an error doing breakpoint commands, we don't
> > +	 want to throw -- continuation might still do something.  */
> > +      TRY_CATCH (e, RETURN_MASK_ERROR)
> > +	{
> > +	  bpstat_do_actions (&stop_bpstat);
> > +	}
> 
> Will things go bad if RETURN_QUIT?  I discovered that we don't handle
> quit very well from some places.
> 
> Easy to trigger; put a command that generates lots of paginated output
> in the commands list and press q at the page prompt.

In fact, things will go bad. I've checked in the following version of the
patch that uses RETURN_MASK_ALL.

- Volodya


[-- Attachment #2: commit.diff --]
[-- Type: text/x-diff, Size: 2131 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.9373
diff -u -p -r1.9373 ChangeLog
--- gdb/ChangeLog	4 May 2008 22:49:47 -0000	1.9373
+++ gdb/ChangeLog	5 May 2008 09:01:57 -0000
@@ -1,3 +1,10 @@
+2008-05-05  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* inf-loop.c (inferior_event_handler): Call bpstat_do_action,
+	and catch all exceptions from it.
+	* top.c (command_line_handler_continuation): Don't
+	call bpstat_do_action here.
+
 2008-05-04  Daniel Jacobowitz  <dan@codesourcery.com>
 
 	* dwarf2read.c (struct dwarf2_cu): Add type_hash.
Index: gdb/inf-loop.c
===================================================================
RCS file: /cvs/src/src/gdb/inf-loop.c,v
retrieving revision 1.13
diff -u -p -r1.13 inf-loop.c
--- gdb/inf-loop.c	24 Apr 2008 11:13:44 -0000	1.13
+++ gdb/inf-loop.c	5 May 2008 09:01:57 -0000
@@ -43,6 +43,7 @@ void
 inferior_event_handler (enum inferior_event_type event_type, 
 			gdb_client_data client_data)
 {
+  struct gdb_exception e;
   int was_sync = 0;
   switch (event_type)
     {
@@ -91,6 +92,12 @@ inferior_event_handler (enum inferior_ev
       was_sync = sync_execution;
       async_enable_stdin ();
 
+      /* If there's an error doing breakpoint commands, we don't
+	 want to throw -- continuation might still do something.  */
+      TRY_CATCH (e, RETURN_MASK_ALL)
+	{
+	  bpstat_do_actions (&stop_bpstat);
+	}
       /* If we were doing a multi-step (eg: step n, next n), but it
 	 got interrupted by a breakpoint, still do the pending
 	 continuations.  The continuation itself is responsible for
Index: gdb/top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.139
diff -u -p -r1.139 top.c
--- gdb/top.c	24 Apr 2008 11:13:44 -0000	1.139
+++ gdb/top.c	5 May 2008 09:01:57 -0000
@@ -379,8 +379,6 @@ command_line_handler_continuation (struc
   if (error)
     return;
 
-  bpstat_do_actions (&stop_bpstat);
-
   if (display_time)
     {
       long cmd_time = get_run_time () - time_at_cmd_start;