From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11150 invoked by alias); 4 Aug 2011 20:11:10 -0000 Received: (qmail 11140 invoked by uid 22791); 4 Aug 2011 20:11:08 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,TW_CP X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 04 Aug 2011 20:10:51 +0000 Received: (qmail 13498 invoked from network); 4 Aug 2011 20:10:50 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 4 Aug 2011 20:10:50 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Eliminate tui_command_loop Date: Thu, 04 Aug 2011 20:11:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-8-generic; KDE/4.6.2; x86_64; ; ) MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201108042110.45405.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-08/txt/msg00084.txt.bz2 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: I found the patch that explains the "catch_errors" change at . which predates TRY_CATCH by a few years: 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 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. */