Re: [continuation args 1/2] Get rid of struct continuation_arg

[Pedro Alves <pedro at codesourcery dot com>]

A Friday 11 July 2008 21:38:01, Pedro Alves wrote:
> This is part #1 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.
>
> I don't think this patch needs much explanation.
>
> Has a nice code size drop, and legibility increase, I think.
>
> Tested on x86_64-unknown-linux-gnu {sync,async}


> +      struct until_break_command_continuation_args *args;
> +      args = xmalloc (sizeof (*arg));

And here's an updated patch, that fixes the dumb typo above.

OK?


-- 
Pedro Alves

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

	Replace struct continuation_args by void* and per command structs.

	* top.c (execute_command): Remove unused arg1 and arg2 locals.

	* breakpoint.c (struct until_break_command_continuation_args):
	New.
	(until_break_command_continuation): Take a void* instead of a
	continuations_arg.  Adjust.
	(until_break_command): Adjust to use struct
	until_break_command_continuation_args instead of struct
	continuation_arg.

	* infcmd.c (struct step_1_continuation_args): New.
	(step_1_continuation): Take a void* instead of a
	continuations_arg.  Adjust to use struct step_1_continuation_args.
	(step_once): Adjust to use struct step_1_continuation_args.

	(struct finish_command_continuation_args): New.
	(finish_command_continuation): Take a void* instead of a
	continuations_arg.  Adjust to use struct
	finish_command_continuation_args.
	(finish_command): Adjust to use struct
	finish_command_continuation_args.
	(struct attach_command_continuation_args): New.
	(attach_command_continuation): Take a void* instead of a
	continuations_arg.  Adjust to use struct
	attach_command_continuation_args.
	(attach_command): Adjust to use struct
	attach_command_continuation_args.

	* defs.h (struct continuation_arg): Delete.
	(struct continuation): Replace the struct continuation_arg*
	parameter of continuation_hook by a void*.  Replace "arg_list"
	member by a new "args" member with void* type.
	(add_continuation, add_intermediate_continuation): Replace struct
	continuation_arg type usages by void* usages.

	* utils.c (add_continuation, do_all_continuations)
	(add_intermediate_continuation)
	(do_all_intermediate_continuations): Replace struct
	continuation_arg type usages by void* usages.  Pass "args" instead
	of "arg_list".

---
 gdb/breakpoint.c |   42 +++++++---------
 gdb/defs.h       |   20 +------
 gdb/infcmd.c     |  141 ++++++++++++++++++++++---------------------------------
 gdb/top.c        |    2 
 gdb/utils.c      |   14 ++---
 5 files changed, 87 insertions(+), 132 deletions(-)

Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2008-07-11 21:02:36.000000000 +0100
+++ src/gdb/top.c	2008-07-11 21:02:38.000000000 +0100
@@ -369,8 +369,6 @@ execute_command (char *p, int from_tty)
   enum language flang;
   static int warned = 0;
   char *line;
-  struct continuation_arg *arg1;
-  struct continuation_arg *arg2;
   long time_at_cmd_start = 0;
 #ifdef HAVE_SBRK
   long space_at_cmd_start = 0;
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-07-11 21:02:36.000000000 +0100
+++ src/gdb/breakpoint.c	2008-07-11 22:17:18.000000000 +0100
@@ -62,8 +62,7 @@
 
 /* Prototypes for local functions. */
 
-static void until_break_command_continuation (struct continuation_arg *arg, 
-					      int error);
+static void until_break_command_continuation (void *arg, int error);
 
 static void catch_command_1 (char *, int, int);
 
@@ -6151,16 +6150,24 @@ awatch_command (char *arg, int from_tty)
 /* Helper routines for the until_command routine in infcmd.c.  Here
    because it uses the mechanisms of breakpoints.  */
 
+struct until_break_command_continuation_args
+{
+  struct breakpoint *breakpoint;
+  struct breakpoint *breakpoint2;
+};
+
 /* This function is called by fetch_inferior_event via the
    cmd_continuation pointer, to complete the until command. It takes
    care of cleaning up the temporary breakpoints set up by the until
    command. */
 static void
-until_break_command_continuation (struct continuation_arg *arg, int error)
+until_break_command_continuation (void *arg, int error)
 {
-  delete_breakpoint ((struct breakpoint *)(arg->data.pointer));
-  if (arg->next)
-    delete_breakpoint ((struct breakpoint *)(arg->next->data.pointer));
+  struct until_break_command_continuation_args *a = arg;
+
+  delete_breakpoint (a->breakpoint);
+  if (a->breakpoint2)
+    delete_breakpoint (a->breakpoint2);
 }
 
 void
@@ -6173,9 +6180,6 @@ until_break_command (char *arg, int from
   struct breakpoint *breakpoint;
   struct breakpoint *breakpoint2 = NULL;
   struct cleanup *old_chain;
-  struct continuation_arg *arg1;
-  struct continuation_arg *arg2;
-
 
   clear_proceed_status ();
 
@@ -6232,22 +6236,14 @@ until_break_command (char *arg, int from
 
   if (target_can_async_p () && is_running (inferior_ptid))
     {
-      arg1 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg1->next         = NULL;
-      arg1->data.pointer = breakpoint;
-
-      if (breakpoint2)
-	{
-	  arg2 = (struct continuation_arg *)
-	    xmalloc ( sizeof (struct continuation_arg));
-	  arg2->next         = NULL;
-	  arg2->data.pointer = breakpoint2;
-	  arg1->next = arg2;	   
-	}
+      struct until_break_command_continuation_args *args;
+      args = xmalloc (sizeof (*args));
+
+      args->breakpoint = breakpoint;
+      args->breakpoint2 = breakpoint2;
 
       discard_cleanups (old_chain);
-      add_continuation (until_break_command_continuation, arg1);
+      add_continuation (until_break_command_continuation, args);
     }
   else
     do_cleanups (old_chain);
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-07-11 21:02:37.000000000 +0100
+++ src/gdb/infcmd.c	2008-07-11 22:16:24.000000000 +0100
@@ -73,8 +73,7 @@ static void nofp_registers_info (char *,
 static void print_return_value (struct type *func_type,
 				struct type *value_type);
 
-static void finish_command_continuation (struct continuation_arg *, 
-					 int error_p);
+static void finish_command_continuation (void *args, int error_p);
 
 static void until_next_command (int);
 
@@ -108,7 +107,7 @@ 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 (struct continuation_arg *arg, int error_p);
+static void step_1_continuation (void *args, int error_p);
 
 static void next_command (char *, int);
 
@@ -810,35 +809,35 @@ which has no line number information.\n"
     }
 }
 
+struct step_1_continuation_args
+{
+  int count;
+  int skip_subroutines;
+  int single_inst;
+  int thread;
+};
+
 /* Called after we are done with one step operation, to check whether
    we need to step again, before we print the prompt and return control
    to the user. If count is > 1, we will need to do one more call to
    proceed(), via step_once(). Basically it is like step_once and
    step_1_continuation are co-recursive. */
 static void
-step_1_continuation (struct continuation_arg *arg, int error_p)
+step_1_continuation (void *args, int error_p)
 {
-  int count;
-  int skip_subroutines;
-  int single_inst;
-  int thread;
-      
-  skip_subroutines = arg->data.integer;
-  single_inst      = arg->next->data.integer;
-  count            = arg->next->next->data.integer;
-  thread           = arg->next->next->next->data.integer;
+  struct step_1_continuation_args *a = args;
 
   if (error_p || !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 (!single_inst || skip_subroutines)
-	delete_longjmp_breakpoint (thread);
+      if (!a->single_inst || a->skip_subroutines)
+	delete_longjmp_breakpoint (a->thread);
       step_multi = 0;
     }
   else
-    step_once (skip_subroutines, single_inst, count - 1, thread);
+    step_once (a->skip_subroutines, a->single_inst, a->count - 1, a->thread);
 }
 
 /* Do just one step operation. If count >1 we will have to set up a
@@ -850,12 +849,9 @@ step_1_continuation (struct continuation
    been completed.*/
 static void 
 step_once (int skip_subroutines, int single_inst, int count, int thread)
-{ 
-  struct continuation_arg *arg1; 
-  struct continuation_arg *arg2;
-  struct continuation_arg *arg3; 
-  struct continuation_arg *arg4;
+{
   struct frame_info *frame;
+  struct step_1_continuation_args *args;
 
   if (count > 0)
     {
@@ -904,23 +900,13 @@ which has no line number information.\n"
 
       step_multi = (count > 1);
       proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 1);
-      arg1 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg2 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg3 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg4 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg1->next = arg2;
-      arg1->data.integer = skip_subroutines;
-      arg2->next = arg3;
-      arg2->data.integer = single_inst;
-      arg3->next = arg4;
-      arg3->data.integer = count;
-      arg4->next = NULL;
-      arg4->data.integer = thread;
-      add_intermediate_continuation (step_1_continuation, arg1);
+
+      args = xmalloc (sizeof (*args));
+      args->skip_subroutines = skip_subroutines;
+      args->single_inst = single_inst;
+      args->count = count;
+      args->thread = thread;
+      add_intermediate_continuation (step_1_continuation, args);
     }
 }
 
@@ -1278,30 +1264,31 @@ print_return_value (struct type *func_ty
    soon as it detects that the target has stopped. This function is
    called via the cmd_continuation pointer.  */
 
-static void
-finish_command_continuation (struct continuation_arg *arg, int error_p)
+struct finish_command_continuation_args
 {
-  struct symbol *function;
   struct breakpoint *breakpoint;
-  struct cleanup *cleanups;
+  struct symbol *function;
+};
 
-  breakpoint = (struct breakpoint *) arg->data.pointer;
-  function = (struct symbol *) arg->next->data.pointer;
+static void
+finish_command_continuation (void *arg, int error_p)
+{
+  struct finish_command_continuation_args *a = arg;
 
   if (!error_p)
     {
-      if (bpstat_find_breakpoint (stop_bpstat, breakpoint) != NULL
-	  && 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 (function));
+
+	  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 (function), value_type); 
+	    print_return_value (SYMBOL_TYPE (a->function), value_type);
 	}
 
       /* We suppress normal call of normal_stop observer and do it here so that
@@ -1310,7 +1297,7 @@ finish_command_continuation (struct cont
     }
 
   suppress_stop_observer = 0;
-  delete_breakpoint (breakpoint);
+  delete_breakpoint (a->breakpoint);
 }
 
 /* "finish": Set a temporary breakpoint at the place the selected
@@ -1324,7 +1311,7 @@ finish_command (char *arg, int from_tty)
   struct symbol *function;
   struct breakpoint *breakpoint;
   struct cleanup *old_chain;
-  struct continuation_arg *arg1, *arg2, *arg3;
+  struct finish_command_continuation_args *cargs;
 
   int async_exec = 0;
 
@@ -1380,16 +1367,12 @@ finish_command (char *arg, int from_tty)
   suppress_stop_observer = 1;
   proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
 
-  arg1 =
-    (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-  arg2 =
-    (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-  arg1->next = arg2;
-  arg2->next = NULL;
-  arg1->data.pointer = breakpoint;
-  arg2->data.pointer = function;
-  add_continuation (finish_command_continuation, arg1);
-  
+  cargs = xmalloc (sizeof (*cargs));
+
+  cargs->breakpoint = breakpoint;
+  cargs->function = function;
+  add_continuation (finish_command_continuation, cargs);
+
   discard_cleanups (old_chain);
   if (!target_can_async_p ())
     do_all_continuations (0);
@@ -1937,18 +1920,18 @@ attach_command_post_wait (char *args, in
     }
 }
 
-static void
-attach_command_continuation (struct continuation_arg *arg, int error_p)
+struct attach_command_continuation_args
 {
   char *args;
   int from_tty;
   int async_exec;
+};
 
-  args = (char *) arg->data.pointer;
-  from_tty = arg->next->data.integer;
-  async_exec = arg->next->next->data.integer;
-
-  attach_command_post_wait (args, from_tty, async_exec);
+static void
+attach_command_continuation (void *args, int error_p)
+{
+  struct attach_command_continuation_args *a = args;
+  attach_command_post_wait (a->args, a->from_tty, a->async_exec);
 }
 
 void
@@ -2033,21 +2016,13 @@ attach_command (char *args, int from_tty
       if (target_can_async_p ())
 	{
 	  /* sync_execution mode.  Wait for stop.  */
-	  struct continuation_arg *arg1, *arg2, *arg3;
+	  struct attach_command_continuation_args *a;
 
-	  arg1 =
-	    (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-	  arg2 =
-	    (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-	  arg3 =
-	    (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-	  arg1->next = arg2;
-	  arg2->next = arg3;
-	  arg3->next = NULL;
-	  arg1->data.pointer = args;
-	  arg2->data.integer = from_tty;
-	  arg3->data.integer = async_exec;
-	  add_continuation (attach_command_continuation, arg1);
+	  a = xmalloc (sizeof (*a));
+	  a->args = xstrdup (args);
+	  a->from_tty = from_tty;
+	  a->async_exec = async_exec;
+	  add_continuation (attach_command_continuation, a);
 	  return;
 	}
 
Index: src/gdb/defs.h
===================================================================
--- src.orig/gdb/defs.h	2008-07-11 21:02:36.000000000 +0100
+++ src/gdb/defs.h	2008-07-11 22:16:23.000000000 +0100
@@ -674,20 +674,10 @@ 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_arg
-  {
-    struct continuation_arg *next;
-    union continuation_data {
-      void *pointer;
-      int   integer;
-      long  longint;
-    } data;
-  };
-
 struct continuation
   {
-    void (*continuation_hook) (struct continuation_arg *, int);
-    struct continuation_arg *arg_list;
+    void (*continuation_hook) (void *, int);
+    void *args;
     struct continuation *next;
   };
 
@@ -697,13 +687,11 @@ extern struct continuation *cmd_continua
 extern struct continuation *intermediate_continuation;
 
 /* From utils.c */
-extern void add_continuation (void (*)(struct continuation_arg *, int),
-			      struct continuation_arg *);
+extern void add_continuation (void (*)(void *, int), void *);
 extern void do_all_continuations (int error);
 extern void discard_all_continuations (void);
 
-extern void add_intermediate_continuation (void (*)(struct continuation_arg *, int),
-			      struct continuation_arg *);
+extern void add_intermediate_continuation (void (*)(void *, int), void *);
 extern void do_all_intermediate_continuations (int error);
 extern void discard_all_intermediate_continuations (void);
 
Index: src/gdb/utils.c
===================================================================
--- src.orig/gdb/utils.c	2008-07-11 21:02:36.000000000 +0100
+++ src/gdb/utils.c	2008-07-11 22:16:23.000000000 +0100
@@ -465,15 +465,14 @@ 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) (struct continuation_arg *, int),
-		  struct continuation_arg *arg_list)
+add_continuation (void (*continuation_hook) (void *, int), void *args)
 {
   struct continuation *continuation_ptr;
 
   continuation_ptr =
     (struct continuation *) xmalloc (sizeof (struct continuation));
   continuation_ptr->continuation_hook = continuation_hook;
-  continuation_ptr->arg_list = arg_list;
+  continuation_ptr->args = args;
   continuation_ptr->next = cmd_continuation;
   cmd_continuation = continuation_ptr;
 }
@@ -502,7 +501,7 @@ do_all_continuations (int error)
   /* Work now on the list we have set aside.  */
   while (continuation_ptr)
     {
-      (continuation_ptr->continuation_hook) (continuation_ptr->arg_list, error);
+      (continuation_ptr->continuation_hook) (continuation_ptr->args, error);
       saved_continuation = continuation_ptr;
       continuation_ptr = continuation_ptr->next;
       xfree (saved_continuation);
@@ -529,15 +528,14 @@ discard_all_continuations (void)
    the front.  */
 void
 add_intermediate_continuation (void (*continuation_hook)
-			       (struct continuation_arg *, int),
-			       struct continuation_arg *arg_list)
+			       (void *, int), void *args)
 {
   struct continuation *continuation_ptr;
 
   continuation_ptr =
     (struct continuation *) xmalloc (sizeof (struct continuation));
   continuation_ptr->continuation_hook = continuation_hook;
-  continuation_ptr->arg_list = arg_list;
+  continuation_ptr->args = args;
   continuation_ptr->next = intermediate_continuation;
   intermediate_continuation = continuation_ptr;
 }
@@ -566,7 +564,7 @@ do_all_intermediate_continuations (int e
   /* Work now on the list we have set aside.  */
   while (continuation_ptr)
     {
-      (continuation_ptr->continuation_hook) (continuation_ptr->arg_list, error);
+      (continuation_ptr->continuation_hook) (continuation_ptr->args, error);
       saved_continuation = continuation_ptr;
       continuation_ptr = continuation_ptr->next;
       xfree (saved_continuation);