[continuation args 2/2] Make continuation args not leak

[continuation args 2/2] Make continuation args not leak

[Pedro Alves]

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

This is part #2 of a two patch series to does two things:

  1) get rid of the crock that was struct continuation_arg, by replacing
     it by a void*, and a structure per continuation.

  2) Make continuations have a mechanism to free the heap allocated
     arguments.

Every time we are adding a continuation, we are allocating its
arguments on the heap.  It happens that currently, it isn't defined
who is responsible to free that, so, nobody is doing it...

I'm adding a new argument to add_continuation so the client now
passes a function to be called either:

 - after the continuation has ran successfully, or,
 - when the continuation is being discarded.

Normally, xfree is passed, but there are some cases that need
to do extra work.

I'm dropping the error_p argument that was passed to the continuation
because:
  
 - it was only called on a really bad error, after poping the current
   execution target.
 - logically, that case is indistinguishable from discarding
   the continuations.

This makes the continuation mechanism very similar to cleanups.  In
effect, they can be implemented on top of cleanups, and get rid
of a bunch of duplication.  So, I've,

- made a struct continuation an opaque type
- internally consider it a cleanup
- reimplemented the continuations manipulation functions (adding,
  running, discarding), on top of the similar cleanup mechanisms.

Tested on x86_64-unknown-linux-gnu {sync,async}.

OK?

-- 
Pedro Alves

[-- Attachment #2: 002-continuations_as_cleanups.diff --]
[-- Type: text/x-diff, Size: 19197 bytes --]

2008-07-11  Pedro Alves  <pedro@codesourcery.com>

	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     |   80 +++++++++++++++++++++++++++--------------------
 gdb/interps.c    |    2 -
 gdb/utils.c      |   93 ++++++++++++++++++++-----------------------------------
 7 files changed, 101 insertions(+), 111 deletions(-)

Index: src/gdb/defs.h
===================================================================
--- src.orig/gdb/defs.h	2008-07-11 21:02:38.000000000 +0100
+++ src/gdb/defs.h	2008-07-11 21:16:58.000000000 +0100
@@ -674,12 +674,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;
@@ -687,12 +682,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-11 21:02:38.000000000 +0100
+++ src/gdb/utils.c	2008-07-11 21:16:58.000000000 +0100
@@ -465,16 +465,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_ftype = 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_ftype,
+		    args,
+		    continuation_free_args);
 }
 
 /* Walk down the cmd_continuation list, and execute all the
@@ -486,26 +486,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
@@ -513,14 +507,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
@@ -528,16 +516,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_ftype = 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_ftype,
+		    args,
+		    continuation_free_args);
 }
 
 /* Walk down the cmd_continuation list, and execute all the
@@ -549,26 +537,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
@@ -576,14 +558,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);
 }
 \f
 
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-07-11 21:02:38.000000000 +0100
+++ src/gdb/breakpoint.c	2008-07-11 21:16:58.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-11 21:02:20.000000000 +0100
+++ src/gdb/inf-loop.c	2008-07-11 21:16:58.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)
@@ -135,7 +137,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-11 21:02:38.000000000 +0100
+++ src/gdb/infcmd.c	2008-07-11 21:16:58.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);
 
@@ -823,15 +820,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;
@@ -906,7 +902,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);
     }
 }
 
@@ -1271,35 +1267,43 @@ 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;
+}
+
 /* "finish": Set a temporary breakpoint at the place the selected
    frame will return to, then continue.  */
 
@@ -1371,11 +1375,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 ();
 }
 \f
 
@@ -1928,12 +1933,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)
 {
@@ -2022,7 +2035,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 21:02:19.000000000 +0100
+++ src/gdb/interps.c	2008-07-11 21:16:58.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 21:02:20.000000000 +0100
+++ src/gdb/event-top.c	2008-07-11 21:16:58.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);
     }

Re: [continuation args 2/2] Make continuation args not leak

[Daniel Jacobowitz]

On Fri, Jul 11, 2008 at 09:38:55PM +0100, Pedro Alves wrote:
> +  struct cleanup **as_cleanup_p = (struct cleanup **) &cmd_continuation;
> +  make_cleanup_ftype *continuation_hook_ftype = continuation_hook;

ftype stands for function type, and our convention is to use it for
types only.  So please don't use it in the name of a variable.
continuation_fn?

> +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;
> +}

Doesn't it still need to call xfree?

Otherwise OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [continuation args 2/2] Make continuation args not leak

[Pedro Alves]

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

A Saturday 12 July 2008 19:54:48, Daniel Jacobowitz wrote:
> On Fri, Jul 11, 2008 at 09:38:55PM +0100, Pedro Alves wrote:
> > +  struct cleanup **as_cleanup_p = (struct cleanup **) &cmd_continuation;
> > +  make_cleanup_ftype *continuation_hook_ftype = continuation_hook;
>
> ftype stands for function type, and our convention is to use it for
> types only.  So please don't use it in the name of a variable.
> continuation_fn?

Ok.  I've fixed both instances.

> > +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;
> > +}
>
> Doesn't it still need to call xfree?
>

Indeed.

> Otherwise OK.

For the archives, attached is what I checked in after another
round of testing.

-- 
Pedro Alves

[-- Attachment #2: 002-continuations_as_cleanups.diff --]
[-- Type: text/x-diff, Size: 19199 bytes --]

2008-07-12  Pedro Alves  <pedro@codesourcery.com>

	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);
 }
 \f
 
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 ();
 }
 \f
 
@@ -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);
     }

Re: [continuation args 2/2] Make continuation args not leak

[Ulrich Weigand]

Pedro Alves wrote:

> 	Rewrite continuations internals on top of cleanups and plug
> 	continuation arguments leaks.

This breaks the build due to violation of the C aliasing rules:

/home/uweigand/fsf/gdb-head/gdb/utils.c: In function 'add_continuation':
/home/uweigand/fsf/gdb-head/gdb/utils.c:479: warning: dereferencing type-punned pointer will break strict-aliasing rules
/home/uweigand/fsf/gdb-head/gdb/utils.c: In function 'discard_all_continuations':
/home/uweigand/fsf/gdb-head/gdb/utils.c:518: warning: dereferencing type-punned pointer will break strict-aliasing rules
/home/uweigand/fsf/gdb-head/gdb/utils.c: In function 'add_intermediate_continuation':
/home/uweigand/fsf/gdb-head/gdb/utils.c:530: warning: dereferencing type-punned pointer will break strict-aliasing rules
/home/uweigand/fsf/gdb-head/gdb/utils.c: In function 'discard_all_intermediate_continuations':
/home/uweigand/fsf/gdb-head/gdb/utils.c:570: warning: dereferencing type-punned pointer will break strict-aliasing rules


> +  struct cleanup **as_cleanup_p = (struct cleanup **) &cmd_continuation;

This may cause a "struct cleanup *" to alias with a "struct continuation *",
which is not allowed according to the C standard.

Why do we still have a "struct continuation" (as nowhere-defined type)?
Shouldn't this just use "struct cleanup" throughout?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [continuation args 2/2] Make continuation args not leak

[Pedro Alves]

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

A Saturday 12 July 2008 21:23:41, Ulrich Weigand wrote:
> Pedro Alves wrote:
> > 	Rewrite continuations internals on top of cleanups and plug
> > 	continuation arguments leaks.
>
> This breaks the build due to violation of the C aliasing rules:
>
> /home/uweigand/fsf/gdb-head/gdb/utils.c: In function 'add_continuation':
> /home/uweigand/fsf/gdb-head/gdb/utils.c:479: warning: dereferencing
> type-punned pointer will break strict-aliasing rules
...

>
> > +  struct cleanup **as_cleanup_p = (struct cleanup **) &cmd_continuation;
>
> This may cause a "struct cleanup *" to alias with a "struct continuation
> *", which is not allowed according to the C standard.
>

Sorry for that.  Missed building at > -O0 to catch these things.

I thought that since the type is not defined, this would not
be a problem, as you can't dereference through cmd_continuation.

> Why do we still have a "struct continuation" (as nowhere-defined type)?
> Shouldn't this just use "struct cleanup" throughout?

Being a cleanup is an implementation detail, I didn't want to make
that fact public.

This fixes the issue for me by making the inheritance explicit.

Tested on x86_64-unknown-linux-gnu async mode.

OK?

-- 
Pedro Alves

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

2008-07-12  Pedro Alves  <pedro@codesourcery.com>

	* utils.c (struct continuation): Define as inheriting struct
	cleanup.
	(add_continuation, do_all_continuations)
	(discard_all_continuations, add_intermediate_continuation)
	(do_all_intermediate_continuations)
	(discard_all_intermediate_continuations): Adjust.

---
 gdb/utils.c |   37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

Index: src/gdb/utils.c
===================================================================
--- src.orig/gdb/utils.c	2008-07-12 22:50:56.000000000 +0100
+++ src/gdb/utils.c	2008-07-12 23:25:40.000000000 +0100
@@ -470,19 +470,29 @@ null_cleanup (void *arg)
 {
 }
 
+/* Continuations are implemented as cleanups internally.  Inherit from
+   cleanups.  */
+struct continuation
+{
+  struct cleanup base;
+};
+
 /* Add a continuation to the continuation list, the global list
-   cmd_continuation. The new continuation will be added at the front.*/
+   cmd_continuation. The new continuation will be added at the
+   front.  */
 void
 add_continuation (void (*continuation_hook) (void *), void *args,
 		  void (*continuation_free_args) (void *))
 {
-  struct cleanup **as_cleanup_p = (struct cleanup **) &cmd_continuation;
+  struct cleanup *as_cleanup = &cmd_continuation->base;
   make_cleanup_ftype *continuation_hook_fn = continuation_hook;
 
-  make_my_cleanup2 (as_cleanup_p,
+  make_my_cleanup2 (&as_cleanup,
 		    continuation_hook_fn,
 		    args,
 		    continuation_free_args);
+
+  cmd_continuation = (struct continuation *) as_cleanup;
 }
 
 /* Walk down the cmd_continuation list, and execute all the
@@ -503,7 +513,7 @@ do_all_continuations (void)
      effect of invoking the continuations and the processing of the
      preexisting continuations will not be affected.  */
 
-  continuation_ptr = (struct cleanup *) cmd_continuation;
+  continuation_ptr = &cmd_continuation->base;
   cmd_continuation = NULL;
 
   /* Work now on the list we have set aside.  */
@@ -515,8 +525,9 @@ do_all_continuations (void)
 void
 discard_all_continuations (void)
 {
-  struct cleanup **continuation_ptr = (struct cleanup **) &cmd_continuation;
-  discard_my_cleanups (continuation_ptr, NULL);
+  struct cleanup *continuation_ptr = &cmd_continuation->base;
+  discard_my_cleanups (&continuation_ptr, NULL);
+  cmd_continuation = NULL;
 }
 
 /* Add a continuation to the continuation list, the global list
@@ -527,13 +538,15 @@ add_intermediate_continuation (void (*co
 			       (void *), void *args,
 			       void (*continuation_free_args) (void *))
 {
-  struct cleanup **as_cleanup_p = (struct cleanup **) &intermediate_continuation;
+  struct cleanup *as_cleanup = &intermediate_continuation->base;
   make_cleanup_ftype *continuation_hook_fn = continuation_hook;
 
-  make_my_cleanup2 (as_cleanup_p,
+  make_my_cleanup2 (&as_cleanup,
 		    continuation_hook_fn,
 		    args,
 		    continuation_free_args);
+
+  intermediate_continuation = (struct continuation *) as_cleanup;
 }
 
 /* Walk down the cmd_continuation list, and execute all the
@@ -554,7 +567,7 @@ do_all_intermediate_continuations (void)
      effect of invoking the continuations and the processing of the
      preexisting continuations will not be affected.  */
 
-  continuation_ptr = (struct cleanup *) intermediate_continuation;
+  continuation_ptr = &intermediate_continuation->base;
   intermediate_continuation = NULL;
 
   /* Work now on the list we have set aside.  */
@@ -566,9 +579,9 @@ do_all_intermediate_continuations (void)
 void
 discard_all_intermediate_continuations (void)
 {
-  struct cleanup **continuation_ptr
-    = (struct cleanup **) &intermediate_continuation;
-  discard_my_cleanups (continuation_ptr, NULL);
+  struct cleanup *continuation_ptr = &intermediate_continuation->base;
+  discard_my_cleanups (&continuation_ptr, NULL);
+  continuation_ptr = NULL;
 }
 \f
 

Re: [continuation args 2/2] Make continuation args not leak

[Daniel Jacobowitz]

On Sat, Jul 12, 2008 at 11:52:43PM +0100, Pedro Alves wrote:
> 2008-07-12  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* utils.c (struct continuation): Define as inheriting struct
> 	cleanup.
> 	(add_continuation, do_all_continuations)
> 	(discard_all_continuations, add_intermediate_continuation)
> 	(do_all_intermediate_continuations)
> 	(discard_all_intermediate_continuations): Adjust.

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [continuation args 2/2] Make continuation args not leak

[Pedro Alves]

On Sunday 13 July 2008 05:27:53, Daniel Jacobowitz wrote:
> On Sat, Jul 12, 2008 at 11:52:43PM +0100, Pedro Alves wrote:
> > 2008-07-12  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* utils.c (struct continuation): Define as inheriting struct
> > 	cleanup.
> > 	(add_continuation, do_all_continuations)
> > 	(discard_all_continuations, add_intermediate_continuation)
> > 	(do_all_intermediate_continuations)
> > 	(discard_all_intermediate_continuations): Adjust.
>
> OK.

Thanks, I've checked it in.

Sorry for the trouble.

-- 
Pedro Alves

Re: [continuation args 2/2] Make continuation args not leak

[Ulrich Weigand]

Pedro Alves wrote:
> On Sunday 13 July 2008 05:27:53, Daniel Jacobowitz wrote:
> > On Sat, Jul 12, 2008 at 11:52:43PM +0100, Pedro Alves wrote:
> > > 2008-07-12  Pedro Alves  <pedro@codesourcery.com>
> > >
> > > 	* utils.c (struct continuation): Define as inheriting struct
> > > 	cleanup.
> > > 	(add_continuation, do_all_continuations)
> > > 	(discard_all_continuations, add_intermediate_continuation)
> > > 	(do_all_intermediate_continuations)
> > > 	(discard_all_intermediate_continuations): Adjust.
> >
> > OK.
> 
> Thanks, I've checked it in.
> 
> Sorry for the trouble.

Thanks, that works for me ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com