Eliminate tui_command_loop

[Pedro Alves <pedro at codesourcery dot com>]

On Wednesday 03 August 2011 19:10:41, Pedro Alves wrote:
> Looking at the differences between tui/tui-interp.c:tui_command_loop
> and event-top.c:cli_event_loop,
> thinking of getting rid of tui_command_loop, the only difference is
> that tui_command_loop inlines event-loop.c:start_event_loop, and
> adds this bit:
> 
> +      /* Update gdb output according to TUI mode.  Since catch_errors
> +         preserves the uiout from changing, this must be done at top
> +         level of event loop.  */
> +      if (tui_active)
> +        current_uiout = tui_out;
> +      else
> +        current_uiout = tui_old_uiout;
> 
> I thought, "why don't we just use TRY_CATCH instead of
> catch_errors then?".  Turns out we can't as is, because it is
> TRY_CATCH itself that preserves the uiout from changing...

TRY_CATCH no longer does that.  I've applied the
patch below that conceptually does:

 - change both start_event_loop and tui_command_loop
   to use TRY_CATCH instead of catch_errors.
 - removes that tui_command_loop code quoted above
 - since tui_command_loop is now just a copy of 
   cli_event_loop, we simply delete it

In practice it just does the equivalent:

 - changes start_event_loop to use TRY_CATCH.
 - removes tui_command_loop

The original patch that added the duplication 
justifies it exactly on the fact that some function
that later ended up as the current catch_errors had been
changed to preserve uiout:
 <http://sourceware.org/ml/gdb-patches/2002-08/msg00866.html>

I found the patch that explains the "catch_errors" change at
 <http://sourceware.org/ml/gdb-patches/2001-08/msg00144.html>.

which predates TRY_CATCH by a few years:
 <http://sourceware.org/ml/gdb/2005-01/msg00064.html>

It seems TRY_CATCH ended up save/restoring uiout
due to the nature of incremental improvements only,
not because it was thought it was required.  Good.

BTW, I tried removing just the

-      /* Update gdb output according to TUI mode.  Since catch_errors
-         preserves the uiout from changing, this must be done at top
-         level of event loop.  */
-      if (tui_active)
-        current_uiout = tui_out;
-      else
-        current_uiout = tui_old_uiout;

bit to see what breaks.  "bt" broke immediately.  The whole screen messes up.

-- 
Pedro Alves

2011-08-04  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* event-loop.c (gdb_do_one_event): Remove `data' parameter.
	(start_event_loop): Use TRY_CATCH instead of catch_errors.
	* event-loop.h (gdb_do_one_event): Remove `data' parameter.
	* top.c (gdb_readline_wrapper): Adjust.
	* tui/tui-interp.c (tui_command_loop):
	(_initialize_tui_interp): Don't install it.

---
 gdb/event-loop.c     |   39 ++++++++++++-------------
 gdb/event-loop.h     |    2 -
 gdb/top.c            |    3 -
 gdb/tui/tui-interp.c |   79 ---------------------------------------------------
 4 files changed, 22 insertions(+), 101 deletions(-)

Index: src/gdb/event-loop.c
===================================================================
--- src.orig/gdb/event-loop.c	2011-08-04 20:20:50.326650779 +0100
+++ src/gdb/event-loop.c	2011-08-04 20:30:29.386650881 +0100
@@ -410,11 +410,10 @@ process_event (void)
 /* Process one high level event.  If nothing is ready at this time,
    wait for something to happen (via gdb_wait_for_event), then process
    it.  Returns >0 if something was done otherwise returns <0 (this
-   can happen if there are no event sources to wait for).  If an error
-   occurs catch_errors() which calls this function returns zero.  */
+   can happen if there are no event sources to wait for).  */
 
 int
-gdb_do_one_event (void *data)
+gdb_do_one_event (void)
 {
   static int event_source_head = 0;
   const int number_of_sources = 3;
@@ -478,30 +477,30 @@ gdb_do_one_event (void *data)
 void
 start_event_loop (void)
 {
-  /* Loop until there is nothing to do.  This is the entry point to the
-     event loop engine.  gdb_do_one_event, called via catch_errors()
-     will process one event for each invocation.  It blocks waits for
-     an event and then processes it.  >0 when an event is processed, 0
-     when catch_errors() caught an error and <0 when there are no
-     longer any event sources registered.  */
+  /* Loop until there is nothing to do.  This is the entry point to
+     the event loop engine.  gdb_do_one_event will process one event
+     for each invocation.  It blocks waiting for an event and then
+     processes it.  */
   while (1)
     {
-      int gdb_result;
+      volatile struct gdb_exception ex;
+      int result = 0;
 
-      gdb_result = catch_errors (gdb_do_one_event, 0, "", RETURN_MASK_ALL);
-      if (gdb_result < 0)
-	break;
-
-      /* If we long-jumped out of do_one_event, we probably
-         didn't get around to resetting the prompt, which leaves
-         readline in a messed-up state.  Reset it here.  */
-
-      if (gdb_result == 0)
+      TRY_CATCH (ex, RETURN_MASK_ALL)
 	{
+	  result = gdb_do_one_event ();
+	}
+      if (ex.reason < 0)
+	{
+	  exception_print (gdb_stderr, ex);
+
 	  /* If any exception escaped to here, we better enable
 	     stdin.  Otherwise, any command that calls async_disable_stdin,
 	     and then throws, will leave stdin inoperable.  */
 	  async_enable_stdin ();
+	  /* If we long-jumped out of do_one_event, we probably didn't
+	     get around to resetting the prompt, which leaves readline
+	     in a messed-up state.  Reset it here.  */
 	  /* FIXME: this should really be a call to a hook that is
 	     interface specific, because interfaces can display the
 	     prompt in their own way.  */
@@ -517,6 +516,8 @@ start_event_loop (void)
 	  /* Maybe better to set a flag to be checked somewhere as to
 	     whether display the prompt or not.  */
 	}
+      if (result < 0)
+	break;
     }
 
   /* We are done with the event loop.  There are no more event sources
Index: src/gdb/event-loop.h
===================================================================
--- src.orig/gdb/event-loop.h	2011-08-04 20:20:50.326650779 +0100
+++ src/gdb/event-loop.h	2011-08-04 20:25:03.256650823 +0100
@@ -91,7 +91,7 @@ queue_position;
 /* Exported functions from event-loop.c */
 
 extern void start_event_loop (void);
-extern int gdb_do_one_event (void *data);
+extern int gdb_do_one_event (void);
 extern void delete_file_handler (int fd);
 extern void add_file_handler (int fd, handler_func *proc, 
 			      gdb_client_data client_data);
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2011-08-04 20:20:50.326650779 +0100
+++ src/gdb/top.c	2011-08-04 20:25:03.256650823 +0100
@@ -797,8 +797,7 @@ gdb_readline_wrapper (char *prompt)
     (*after_char_processing_hook) ();
   gdb_assert (after_char_processing_hook == NULL);
 
-  /* gdb_do_one_event argument is unused.  */
-  while (gdb_do_one_event (NULL) >= 0)
+  while (gdb_do_one_event () >= 0)
     if (gdb_readline_wrapper_done)
       break;
 
Index: src/gdb/tui/tui-interp.c
===================================================================
--- src.orig/gdb/tui/tui-interp.c	2011-08-04 20:25:02.046650823 +0100
+++ src/gdb/tui/tui-interp.c	2011-08-04 20:25:03.256650823 +0100
@@ -132,84 +132,6 @@ tui_exec (void *data, const char *comman
   internal_error (__FILE__, __LINE__, _("tui_exec called"));
 }
 
-
-/* Initialize all the necessary variables, start the event loop,
-   register readline, and stdin, start the loop.  */
-
-static void
-tui_command_loop (void *data)
-{
-  /* If we are using readline, set things up and display the first
-     prompt, otherwise just print the prompt.  */
-  if (async_command_editing_p)
-    {
-      int length;
-      char *a_prompt;
-      char *gdb_prompt = get_prompt (0);
-
-      /* Tell readline what the prompt to display is and what function
-         it will need to call after a whole line is read. This also
-         displays the first prompt.  */
-      length = strlen (get_prefix (0))
-	+ strlen (gdb_prompt) + strlen (get_suffix (0)) + 1;
-      a_prompt = (char *) alloca (length);
-      strcpy (a_prompt, get_prefix (0));
-      strcat (a_prompt, gdb_prompt);
-      strcat (a_prompt, get_suffix (0));
-      rl_callback_handler_install (a_prompt, input_handler);
-    }
-  else
-    display_gdb_prompt (0);
-
-  /* Loop until there is nothing to do. This is the entry point to the
-     event loop engine. gdb_do_one_event, called via catch_errors()
-     will process one event for each invocation.  It blocks waits for
-     an event and then processes it.  >0 when an event is processed, 0
-     when catch_errors() caught an error and <0 when there are no
-     longer any event sources registered.  */
-  while (1)
-    {
-      int result = catch_errors (gdb_do_one_event, 0, "", RETURN_MASK_ALL);
-
-      if (result < 0)
-	break;
-
-      /* Update gdb output according to TUI mode.  Since catch_errors
-         preserves the uiout from changing, this must be done at top
-         level of event loop.  */
-      if (tui_active)
-        current_uiout = tui_out;
-      else
-        current_uiout = tui_old_uiout;
-
-      if (result == 0)
-	{
-	  /* If any exception escaped to here, we better enable
-	     stdin.  Otherwise, any command that calls async_disable_stdin,
-	     and then throws, will leave stdin inoperable.  */
-	  async_enable_stdin ();
-	  /* FIXME: this should really be a call to a hook that is
-	     interface specific, because interfaces can display the
-	     prompt in their own way.  */
-	  display_gdb_prompt (0);
-	  /* This call looks bizarre, but it is required.  If the user
-	     entered a command that caused an error,
-	     after_char_processing_hook won't be called from
-	     rl_callback_read_char_wrapper.  Using a cleanup there
-	     won't work, since we want this function to be called
-	     after a new prompt is printed.  */
-	  if (after_char_processing_hook)
-	    (*after_char_processing_hook) ();
-	  /* Maybe better to set a flag to be checked somewhere as to
-	     whether display the prompt or not.  */
-	}
-    }
-
-  /* We are done with the event loop. There are no more event sources
-     to listen to.  So we exit GDB.  */
-  return;
-}
-
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_tui_interp;
 
@@ -222,7 +144,6 @@ _initialize_tui_interp (void)
     tui_suspend,
     tui_exec,
     tui_display_prompt_p,
-    tui_command_loop,
   };
 
   /* Create a default uiout builder for the TUI.  */