2008-07-12 Pedro Alves Rewrite continuations internals on top of cleanups and plug continuation arguments leaks. * defs.h (struct continuation): Make it opaque. (add_continuation, add_intermediate_continuation): Drop the int argument of the continuation hook argument. Add continuation_free_args argument. (do_all_continuations, do_all_intermediate_continuations): Drop the error_p argument. * utils.c (add_continuation): Drop the int argument of the continuation hook argument. Add continuation_free_args argument. Reimplement on top of cleanups. (do_all_continuations): Drop error argument. Reimplement on top of cleanups. (discard_all_continuations): Reimplement on top of cleanups. (add_intermediate_continuation): Drop the int argument of the continuation hook argument. Add continuation_free_args argument. Reimplement on top of cleanups. (do_all_intermediate_continuations): Drop error argument. Reimplement on top of cleanups. (discard_all_intermediate_continuations): Reimplement on top of cleanups. * breakpoint.c (until_break_command_continuation): Drop error argument. Add xfree as continuation argument deleter. * inf-loop.c (inferior_event_handler): On error, discard all continuations. Adjust to new do_all_intermediate_continuations and do_all_continuations interfaces. * infcmd.c (step_1_continuation): Drop error_p argument. Adjust. Pass xfree as continuation argument deleter. (finish_command_continuation): Drop error_p argument. Adjust. (finish_command_continuation_free_arg): New. (finish_command): Pass finish_command_continuation_free_arg as continuation argument deleter. Adjust to new do_all_continuations interfaces. (attach_command_continuation): Drop error_p argument. (attach_command_continuation_free_args): New. (attach_command): Pass attach_command_continuation_free_args as continuation argument deleter. * interps.c (interp_set): Adjust to new do_all_continuations interfaces. * event-top.c (stdin_event_handler): In error, also discard the intermediate continuations. --- gdb/breakpoint.c | 7 +--- gdb/defs.h | 17 ++++------ gdb/event-top.c | 1 gdb/inf-loop.c | 12 ++++--- gdb/infcmd.c | 81 ++++++++++++++++++++++++++++------------------- gdb/interps.c | 2 - gdb/utils.c | 93 ++++++++++++++++++++----------------------------------- 7 files changed, 102 insertions(+), 111 deletions(-) Index: src/gdb/defs.h =================================================================== --- src.orig/gdb/defs.h 2008-07-12 20:08:48.000000000 +0100 +++ src/gdb/defs.h 2008-07-12 20:08:56.000000000 +0100 @@ -677,12 +677,7 @@ extern void free_command_lines (struct c used by the finish and until commands, and in the remote protocol when opening an extended-remote connection. */ -struct continuation - { - void (*continuation_hook) (void *, int); - void *args; - struct continuation *next; - }; +struct continuation; /* In infrun.c. */ extern struct continuation *cmd_continuation; @@ -690,12 +685,14 @@ extern struct continuation *cmd_continua extern struct continuation *intermediate_continuation; /* From utils.c */ -extern void add_continuation (void (*)(void *, int), void *); -extern void do_all_continuations (int error); +extern void add_continuation (void (*)(void *), void *, + void (*)(void *)); +extern void do_all_continuations (void); extern void discard_all_continuations (void); -extern void add_intermediate_continuation (void (*)(void *, int), void *); -extern void do_all_intermediate_continuations (int error); +extern void add_intermediate_continuation (void (*)(void *), void *, + void (*)(void *)); +extern void do_all_intermediate_continuations (void); extern void discard_all_intermediate_continuations (void); /* String containing the current directory (what getwd would return). */ Index: src/gdb/utils.c =================================================================== --- src.orig/gdb/utils.c 2008-07-12 20:08:49.000000000 +0100 +++ src/gdb/utils.c 2008-07-12 20:12:41.000000000 +0100 @@ -473,16 +473,16 @@ null_cleanup (void *arg) /* Add a continuation to the continuation list, the global list cmd_continuation. The new continuation will be added at the front.*/ void -add_continuation (void (*continuation_hook) (void *, int), void *args) +add_continuation (void (*continuation_hook) (void *), void *args, + void (*continuation_free_args) (void *)) { - struct continuation *continuation_ptr; + struct cleanup **as_cleanup_p = (struct cleanup **) &cmd_continuation; + make_cleanup_ftype *continuation_hook_fn = continuation_hook; - continuation_ptr = - (struct continuation *) xmalloc (sizeof (struct continuation)); - continuation_ptr->continuation_hook = continuation_hook; - continuation_ptr->args = args; - continuation_ptr->next = cmd_continuation; - cmd_continuation = continuation_ptr; + make_my_cleanup2 (as_cleanup_p, + continuation_hook_fn, + args, + continuation_free_args); } /* Walk down the cmd_continuation list, and execute all the @@ -494,26 +494,20 @@ add_continuation (void (*continuation_ho and do the continuations from there on, instead of using the global beginning of list as our iteration pointer. */ void -do_all_continuations (int error) +do_all_continuations (void) { - struct continuation *continuation_ptr; - struct continuation *saved_continuation; + struct cleanup *continuation_ptr; /* Copy the list header into another pointer, and set the global list header to null, so that the global list can change as a side - effect of invoking the continuations and the processing of - the preexisting continuations will not be affected. */ - continuation_ptr = cmd_continuation; + effect of invoking the continuations and the processing of the + preexisting continuations will not be affected. */ + + continuation_ptr = (struct cleanup *) cmd_continuation; cmd_continuation = NULL; /* Work now on the list we have set aside. */ - while (continuation_ptr) - { - (continuation_ptr->continuation_hook) (continuation_ptr->args, error); - saved_continuation = continuation_ptr; - continuation_ptr = continuation_ptr->next; - xfree (saved_continuation); - } + do_my_cleanups (&continuation_ptr, NULL); } /* Walk down the cmd_continuation list, and get rid of all the @@ -521,14 +515,8 @@ do_all_continuations (int error) void discard_all_continuations (void) { - struct continuation *continuation_ptr; - - while (cmd_continuation) - { - continuation_ptr = cmd_continuation; - cmd_continuation = continuation_ptr->next; - xfree (continuation_ptr); - } + struct cleanup **continuation_ptr = (struct cleanup **) &cmd_continuation; + discard_my_cleanups (continuation_ptr, NULL); } /* Add a continuation to the continuation list, the global list @@ -536,16 +524,16 @@ discard_all_continuations (void) the front. */ void add_intermediate_continuation (void (*continuation_hook) - (void *, int), void *args) + (void *), void *args, + void (*continuation_free_args) (void *)) { - struct continuation *continuation_ptr; + struct cleanup **as_cleanup_p = (struct cleanup **) &intermediate_continuation; + make_cleanup_ftype *continuation_hook_fn = continuation_hook; - continuation_ptr = - (struct continuation *) xmalloc (sizeof (struct continuation)); - continuation_ptr->continuation_hook = continuation_hook; - continuation_ptr->args = args; - continuation_ptr->next = intermediate_continuation; - intermediate_continuation = continuation_ptr; + make_my_cleanup2 (as_cleanup_p, + continuation_hook_fn, + args, + continuation_free_args); } /* Walk down the cmd_continuation list, and execute all the @@ -557,26 +545,20 @@ add_intermediate_continuation (void (*co and do the continuations from there on, instead of using the global beginning of list as our iteration pointer.*/ void -do_all_intermediate_continuations (int error) +do_all_intermediate_continuations (void) { - struct continuation *continuation_ptr; - struct continuation *saved_continuation; + struct cleanup *continuation_ptr; /* Copy the list header into another pointer, and set the global list header to null, so that the global list can change as a side - effect of invoking the continuations and the processing of - the preexisting continuations will not be affected. */ - continuation_ptr = intermediate_continuation; + effect of invoking the continuations and the processing of the + preexisting continuations will not be affected. */ + + continuation_ptr = (struct cleanup *) intermediate_continuation; intermediate_continuation = NULL; /* Work now on the list we have set aside. */ - while (continuation_ptr) - { - (continuation_ptr->continuation_hook) (continuation_ptr->args, error); - saved_continuation = continuation_ptr; - continuation_ptr = continuation_ptr->next; - xfree (saved_continuation); - } + do_my_cleanups (&continuation_ptr, NULL); } /* Walk down the cmd_continuation list, and get rid of all the @@ -584,14 +566,9 @@ do_all_intermediate_continuations (int e void discard_all_intermediate_continuations (void) { - struct continuation *continuation_ptr; - - while (intermediate_continuation) - { - continuation_ptr = intermediate_continuation; - intermediate_continuation = continuation_ptr->next; - xfree (continuation_ptr); - } + struct cleanup **continuation_ptr + = (struct cleanup **) &intermediate_continuation; + discard_my_cleanups (continuation_ptr, NULL); } Index: src/gdb/breakpoint.c =================================================================== --- src.orig/gdb/breakpoint.c 2008-07-12 20:08:48.000000000 +0100 +++ src/gdb/breakpoint.c 2008-07-12 20:08:56.000000000 +0100 @@ -62,8 +62,6 @@ /* Prototypes for local functions. */ -static void until_break_command_continuation (void *arg, int error); - static void catch_command_1 (char *, int, int); static void enable_delete_command (char *, int); @@ -6161,7 +6159,7 @@ struct until_break_command_continuation_ care of cleaning up the temporary breakpoints set up by the until command. */ static void -until_break_command_continuation (void *arg, int error) +until_break_command_continuation (void *arg) { struct until_break_command_continuation_args *a = arg; @@ -6243,7 +6241,8 @@ until_break_command (char *arg, int from args->breakpoint2 = breakpoint2; discard_cleanups (old_chain); - add_continuation (until_break_command_continuation, args); + add_continuation (until_break_command_continuation, args, + xfree); } else do_cleanups (old_chain); Index: src/gdb/inf-loop.c =================================================================== --- src.orig/gdb/inf-loop.c 2008-07-12 20:02:57.000000000 +0100 +++ src/gdb/inf-loop.c 2008-07-12 20:08:56.000000000 +0100 @@ -52,7 +52,8 @@ inferior_event_handler (enum inferior_ev printf_unfiltered (_("error detected from target.\n")); target_async (NULL, 0); pop_target (); - do_all_continuations (1); + discard_all_intermediate_continuations (); + discard_all_continuations (); async_enable_stdin (); break; @@ -66,7 +67,8 @@ inferior_event_handler (enum inferior_ev { target_async (NULL, 0); pop_target (); - do_all_continuations (1); + discard_all_intermediate_continuations (); + discard_all_continuations (); async_enable_stdin (); display_gdb_prompt (0); } @@ -96,13 +98,13 @@ inferior_event_handler (enum inferior_ev touch the inferior memory, e.g. to remove breakpoints, so run them before running breakpoint commands, which may resume the target. */ - do_all_intermediate_continuations (0); + do_all_intermediate_continuations (); /* Always finish the previous command before running any breakpoint commands. Any stop cancels the previous command. E.g. a "finish" or "step-n" command interrupted by an unrelated breakpoint is canceled. */ - do_all_continuations (0); + do_all_continuations (); if (current_language != expected_language && language_mode == language_mode_auto) @@ -123,7 +125,7 @@ inferior_event_handler (enum inferior_ev case INF_EXEC_CONTINUE: /* Is there anything left to do for the command issued to complete? */ - do_all_intermediate_continuations (0); + do_all_intermediate_continuations (); break; case INF_QUIT_REQ: Index: src/gdb/infcmd.c =================================================================== --- src.orig/gdb/infcmd.c 2008-07-12 20:08:48.000000000 +0100 +++ src/gdb/infcmd.c 2008-07-12 20:10:20.000000000 +0100 @@ -73,8 +73,6 @@ static void nofp_registers_info (char *, static void print_return_value (struct type *func_type, struct type *value_type); -static void finish_command_continuation (void *args, int error_p); - static void until_next_command (int); static void until_command (char *, int); @@ -107,7 +105,6 @@ static void jump_command (char *, int); static void step_1 (int, int, char *); static void step_once (int skip_subroutines, int single_inst, int count, int thread); -static void step_1_continuation (void *args, int error_p); static void next_command (char *, int); @@ -877,15 +874,14 @@ struct step_1_continuation_args proceed(), via step_once(). Basically it is like step_once and step_1_continuation are co-recursive. */ static void -step_1_continuation (void *args, int error_p) +step_1_continuation (void *args) { struct step_1_continuation_args *a = args; - if (error_p || !step_multi || !stop_step) + if (!step_multi || !stop_step) { - /* We either hit an error, or stopped for some reason - that is not stepping, or there are no further steps - to make. Cleanup. */ + /* If we stopped for some reason that is not stepping there are + no further steps to make. Cleanup. */ if (!a->single_inst || a->skip_subroutines) delete_longjmp_breakpoint (a->thread); step_multi = 0; @@ -960,7 +956,7 @@ which has no line number information.\n" args->single_inst = single_inst; args->count = count; args->thread = thread; - add_intermediate_continuation (step_1_continuation, args); + add_intermediate_continuation (step_1_continuation, args, xfree); } } @@ -1325,35 +1321,44 @@ struct finish_command_continuation_args }; static void -finish_command_continuation (void *arg, int error_p) +finish_command_continuation (void *arg) { struct finish_command_continuation_args *a = arg; - if (!error_p) + if (bpstat_find_breakpoint (stop_bpstat, a->breakpoint) != NULL + && a->function != NULL) { - if (bpstat_find_breakpoint (stop_bpstat, a->breakpoint) != NULL - && a->function != NULL) - { - struct type *value_type; - - value_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (a->function)); - if (!value_type) - internal_error (__FILE__, __LINE__, - _("finish_command: function has no target type")); - - if (TYPE_CODE (value_type) != TYPE_CODE_VOID) - print_return_value (SYMBOL_TYPE (a->function), value_type); - } - - /* We suppress normal call of normal_stop observer and do it here so that - that *stopped notification includes the return value. */ - observer_notify_normal_stop (stop_bpstat); - } + struct type *value_type; + value_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (a->function)); + if (!value_type) + internal_error (__FILE__, __LINE__, + _("finish_command: function has no target type")); + + if (TYPE_CODE (value_type) != TYPE_CODE_VOID) + print_return_value (SYMBOL_TYPE (a->function), value_type); + } + + /* We suppress normal call of normal_stop observer and do it here so + that that *stopped notification includes the return value. */ + /* NOTE: This is broken in non-stop mode. There is no guarantee the + next stop will be in the same thread that we started doing a + finish on. This suppressing (or some other replacement means) + should be a thread property. */ + observer_notify_normal_stop (stop_bpstat); suppress_stop_observer = 0; delete_breakpoint (a->breakpoint); } +static void +finish_command_continuation_free_arg (void *arg) +{ + /* NOTE: See finish_command_continuation. This would go away, if + this suppressing is made a thread property. */ + suppress_stop_observer = 0; + xfree (arg); +} + /* "finish": Set a temporary breakpoint at the place the selected frame will return to, then continue. */ @@ -1425,11 +1430,12 @@ finish_command (char *arg, int from_tty) cargs->breakpoint = breakpoint; cargs->function = function; - add_continuation (finish_command_continuation, cargs); + add_continuation (finish_command_continuation, cargs, + finish_command_continuation_free_arg); discard_cleanups (old_chain); if (!target_can_async_p ()) - do_all_continuations (0); + do_all_continuations (); } @@ -1982,12 +1988,20 @@ struct attach_command_continuation_args }; static void -attach_command_continuation (void *args, int error_p) +attach_command_continuation (void *args) { struct attach_command_continuation_args *a = args; attach_command_post_wait (a->args, a->from_tty, a->async_exec); } +static void +attach_command_continuation_free_args (void *args) +{ + struct attach_command_continuation_args *a = args; + xfree (a->args); + xfree (a); +} + void attach_command (char *args, int from_tty) { @@ -2076,7 +2090,8 @@ attach_command (char *args, int from_tty a->args = xstrdup (args); a->from_tty = from_tty; a->async_exec = async_exec; - add_continuation (attach_command_continuation, a); + add_continuation (attach_command_continuation, a, + attach_command_continuation_free_args); return; } Index: src/gdb/interps.c =================================================================== --- src.orig/gdb/interps.c 2008-07-11 22:19:10.000000000 +0100 +++ src/gdb/interps.c 2008-07-12 20:08:56.000000000 +0100 @@ -148,7 +148,7 @@ interp_set (struct interp *interp, int t if (current_interpreter != NULL) { - do_all_continuations (0); + do_all_continuations (); ui_out_flush (uiout); if (current_interpreter->procs->suspend_proc && !current_interpreter->procs->suspend_proc (current_interpreter-> Index: src/gdb/event-top.c =================================================================== --- src.orig/gdb/event-top.c 2008-07-11 22:19:10.000000000 +0100 +++ src/gdb/event-top.c 2008-07-12 20:08:56.000000000 +0100 @@ -425,6 +425,7 @@ stdin_event_handler (int error, gdb_clie printf_unfiltered (_("error detected on stdin\n")); delete_file_handler (input_fd); discard_all_continuations (); + discard_all_intermediate_continuations (); /* If stdin died, we may as well kill gdb. */ quit_command ((char *) 0, stdin == instream); }