[PATCH] varobj: Cleanup dead code

[PATCH] varobj: Cleanup dead code

[Simon Marchi]

This patch removes some dead code.

I noticed that varobj_delete was always called with dellist == NULL, so
I started removing that parameter.  That allows removing a good chunk of
the code in varobj_delete, making it almost trivial.  We can also remove
the resultp parameters in that whole trail.  In turn, this shows that
struct cpstack, cppush and cppop were only used fo that mechanism, so
they can be removed as well.

I also moved the function comment to the header file to comply with
today's guideline, even though the rest of the file does not respect it
(yet).

gdb/ChangeLog:

	* varobj.h (varobj_delete): Remove dellist parameter, update and
	move documentation here.
	* varobj.c (struct cpstack, cppush, cppop): Remove.
	(delete_variable): Remove resultp (first) parameter.
	(delete_variable_1): Likewise.
	(varobj_delete): Remove dellist parameter and unused code.
	(update_dynamic_varobj_children): Adjust varobj_delete call.
	(update_type_if_necessary): Likewise.
	(varobj_set_visualizer): Likewise.
	(varobj_update): Likewise.
	(value_of_root): Likewise.
	(varobj_invalidate_iter): Likewise.
	* mi/mi-cmd-var.c (mi_cmd_var_delete): Likewise.
---
 gdb/mi/mi-cmd-var.c |   2 +-
 gdb/varobj.c        | 119 ++++++++--------------------------------------------
 gdb/varobj.h        |   6 ++-
 3 files changed, 23 insertions(+), 104 deletions(-)

diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 04f326c..3bfe4f0 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -205,7 +205,7 @@ mi_cmd_var_delete (char *command, char **argv, int argc)
 
   var = varobj_get_handle (name);
 
-  numdel = varobj_delete (var, NULL, children_only_p);
+  numdel = varobj_delete (var, children_only_p);
 
   ui_out_field_int (uiout, "ndeleted", numdel);
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index b846ac0..6f56cba 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -136,12 +136,6 @@ struct varobj_dynamic
   varobj_item *saved_item;
 };
 
-struct cpstack
-{
-  char *name;
-  struct cpstack *next;
-};
-
 /* A list of varobjs */
 
 struct vlist
@@ -154,10 +148,9 @@ struct vlist
 
 /* Helper functions for the above subcommands.  */
 
-static int delete_variable (struct cpstack **, struct varobj *, int);
+static int delete_variable (struct varobj *, int);
 
-static void delete_variable_1 (struct cpstack **, int *,
-			       struct varobj *, int, int);
+static void delete_variable_1 (int *, struct varobj *, int, int);
 
 static int install_variable (struct varobj *);
 
@@ -181,10 +174,6 @@ static struct cleanup *make_cleanup_free_variable (struct varobj *var);
 
 static enum varobj_display_formats variable_default_display (struct varobj *);
 
-static void cppush (struct cpstack **pstack, char *name);
-
-static char *cppop (struct cpstack **pstack);
-
 static int update_type_if_necessary (struct varobj *var,
 				     struct value *new_value);
 
@@ -502,51 +491,12 @@ varobj_get_expression (const struct varobj *var)
   return name_of_variable (var);
 }
 
-/* Deletes a varobj and all its children if only_children == 0,
-   otherwise deletes only the children. If DELLIST is non-NULL, it is
-   assigned a malloc'ed list of all the (malloc'ed) names of the variables
-   that have been deleted (NULL terminated).  Returns the number of deleted
-   variables.  */
+/* See varobj.h.  */
 
 int
-varobj_delete (struct varobj *var, char ***dellist, int only_children)
+varobj_delete (struct varobj *var, int only_children)
 {
-  int delcount;
-  int mycount;
-  struct cpstack *result = NULL;
-  char **cp;
-
-  /* Initialize a stack for temporary results.  */
-  cppush (&result, NULL);
-
-  if (only_children)
-    /* Delete only the variable children.  */
-    delcount = delete_variable (&result, var, 1 /* only the children */ );
-  else
-    /* Delete the variable and all its children.  */
-    delcount = delete_variable (&result, var, 0 /* parent+children */ );
-
-  /* We may have been asked to return a list of what has been deleted.  */
-  if (dellist != NULL)
-    {
-      *dellist = XNEWVEC (char *, delcount + 1);
-
-      cp = *dellist;
-      mycount = delcount;
-      *cp = cppop (&result);
-      while ((*cp != NULL) && (mycount > 0))
-	{
-	  mycount--;
-	  cp++;
-	  *cp = cppop (&result);
-	}
-
-      if (mycount || (*cp != NULL))
-	warning (_("varobj_delete: assertion failed - mycount(=%d) <> 0"),
-		 mycount);
-    }
-
-  return delcount;
+  return delete_variable (var, only_children);
 }
 
 #if HAVE_PYTHON
@@ -876,7 +826,7 @@ update_dynamic_varobj_children (struct varobj *var,
 
       *cchanged = 1;
       for (j = i; j < VEC_length (varobj_p, var->children); ++j)
-	varobj_delete (VEC_index (varobj_p, var->children, j), NULL, 0);
+	varobj_delete (VEC_index (varobj_p, var->children, j), 0);
       VEC_truncate (varobj_p, var->children, i);
     }
 
@@ -1313,7 +1263,7 @@ update_type_if_necessary (struct varobj *var, struct value *new_value)
 	      var->type = new_type;
 
 	      /* This information may be not valid for a new type.  */
-	      varobj_delete (var, NULL, 1);
+	      varobj_delete (var, 1);
 	      VEC_free (varobj_p, var->children);
 	      var->num_children = -1;
 	      return 1;
@@ -1569,7 +1519,7 @@ varobj_set_visualizer (struct varobj *var, const char *visualizer)
   Py_XDECREF (constructor);
 
   /* If there are any children now, wipe them.  */
-  varobj_delete (var, NULL, 1 /* children only */);
+  varobj_delete (var, 1 /* children only */);
   var->num_children = -1;
 
   do_cleanups (back_to);
@@ -1720,7 +1670,7 @@ varobj_update (struct varobj **varp, int is_explicit)
 	    {
 	      /* The children are no longer valid; delete them now.
 	         Report the fact that its type changed as well.  */
-	      varobj_delete (v, NULL, 1 /* only_children */);
+	      varobj_delete (v, 1 /* only_children */);
 	      v->num_children = -1;
 	      v->to = -1;
 	      v->from = -1;
@@ -1868,13 +1818,12 @@ varobj_update (struct varobj **varp, int is_explicit)
  */
 
 static int
-delete_variable (struct cpstack **resultp, struct varobj *var,
-		 int only_children_p)
+delete_variable (struct varobj *var, int only_children_p)
 {
   int delcount = 0;
 
-  delete_variable_1 (resultp, &delcount, var,
-		     only_children_p, 1 /* remove_from_parent_p */ );
+  delete_variable_1 (&delcount, var, only_children_p,
+		     1 /* remove_from_parent_p */ );
 
   return delcount;
 }
@@ -1884,8 +1833,7 @@ delete_variable (struct cpstack **resultp, struct varobj *var,
    and the parent is not removed we dump core.  It must be always
    initially called with remove_from_parent_p set.  */
 static void
-delete_variable_1 (struct cpstack **resultp, int *delcountp,
-		   struct varobj *var, int only_children_p,
+delete_variable_1 (int *delcountp, struct varobj *var, int only_children_p,
 		   int remove_from_parent_p)
 {
   int i;
@@ -1899,7 +1847,7 @@ delete_variable_1 (struct cpstack **resultp, int *delcountp,
 	continue;
       if (!remove_from_parent_p)
 	child->parent = NULL;
-      delete_variable_1 (resultp, delcountp, child, 0, only_children_p);
+      delete_variable_1 (delcountp, child, 0, only_children_p);
     }
   VEC_free (varobj_p, var->children);
 
@@ -1912,7 +1860,6 @@ delete_variable_1 (struct cpstack **resultp, int *delcountp,
      yet been installed, don't report it, it belongs to the caller...  */
   if (var->obj_name != NULL)
     {
-      cppush (resultp, xstrdup (var->obj_name));
       *delcountp = *delcountp + 1;
     }
 
@@ -2245,36 +2192,6 @@ variable_default_display (struct varobj *var)
   return FORMAT_NATURAL;
 }
 
-/* FIXME: The following should be generic for any pointer.  */
-static void
-cppush (struct cpstack **pstack, char *name)
-{
-  struct cpstack *s;
-
-  s = XNEW (struct cpstack);
-  s->name = name;
-  s->next = *pstack;
-  *pstack = s;
-}
-
-/* FIXME: The following should be generic for any pointer.  */
-static char *
-cppop (struct cpstack **pstack)
-{
-  struct cpstack *s;
-  char *v;
-
-  if ((*pstack)->name == NULL && (*pstack)->next == NULL)
-    return NULL;
-
-  s = *pstack;
-  v = s->name;
-  *pstack = (*pstack)->next;
-  xfree (s);
-
-  return v;
-}
-\f
 /*
  * Language-dependencies
  */
@@ -2443,7 +2360,7 @@ value_of_root (struct varobj **var_handle, int *type_changed)
          var->root->exp = tmp_var->root->exp;
          tmp_var->root->exp = tmp_exp;
 
-	  varobj_delete (tmp_var, NULL, 0);
+	  varobj_delete (tmp_var, 0);
 	  *type_changed = 0;
 	}
       else
@@ -2451,7 +2368,7 @@ value_of_root (struct varobj **var_handle, int *type_changed)
 	  tmp_var->obj_name = xstrdup (var->obj_name);
 	  tmp_var->from = var->from;
 	  tmp_var->to = var->to;
-	  varobj_delete (var, NULL, 0);
+	  varobj_delete (var, 0);
 
 	  install_variable (tmp_var);
 	  *var_handle = tmp_var;
@@ -2480,7 +2397,7 @@ value_of_root (struct varobj **var_handle, int *type_changed)
 	/* The type has mutated, so the children are no longer valid.
 	   Just delete them, and tell our caller that the type has
 	   changed.  */
-	varobj_delete (var, NULL, 1 /* only_children */);
+	varobj_delete (var, 1 /* only_children */);
 	var->num_children = -1;
 	var->to = -1;
 	var->from = -1;
@@ -2769,7 +2686,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
       if (tmp_var != NULL) 
 	{ 
 	  tmp_var->obj_name = xstrdup (var->obj_name);
-	  varobj_delete (var, NULL, 0);
+	  varobj_delete (var, 0);
 	  install_variable (tmp_var);
 	}
       else
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 66399fd..f750482 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -246,8 +246,10 @@ extern char *varobj_get_objname (const struct varobj *var);
 
 extern char *varobj_get_expression (const struct varobj *var);
 
-extern int varobj_delete (struct varobj *var, char ***dellist,
-			  int only_children);
+/* Delete a varobj and all its children if only_children == 0, otherwise delete
+   only the children.  Return the number of deleted variables.  */
+
+extern int varobj_delete (struct varobj *var, int only_children);
 
 extern enum varobj_display_formats varobj_set_display_format (
 							 struct varobj *var,
-- 
2.7.0

Re: [PATCH] varobj: Cleanup dead code

[Joel Brobecker]

> I also moved the function comment to the header file to comply with
> today's guideline, even though the rest of the file does not respect it
> (yet).

FTR: There is actually no real agreement on this, and because each
approach has its advantages, short of a royal edict, we'll probably
continue accepting both forms.

> gdb/ChangeLog:
> 
> 	* varobj.h (varobj_delete): Remove dellist parameter, update and
> 	move documentation here.
> 	* varobj.c (struct cpstack, cppush, cppop): Remove.
> 	(delete_variable): Remove resultp (first) parameter.
> 	(delete_variable_1): Likewise.
> 	(varobj_delete): Remove dellist parameter and unused code.
> 	(update_dynamic_varobj_children): Adjust varobj_delete call.
> 	(update_type_if_necessary): Likewise.
> 	(varobj_set_visualizer): Likewise.
> 	(varobj_update): Likewise.
> 	(value_of_root): Likewise.
> 	(varobj_invalidate_iter): Likewise.
> 	* mi/mi-cmd-var.c (mi_cmd_var_delete): Likewise.

Looks good to me.

Thanks for doing that cleanup.
-- 
Joel

Re: [PATCH] varobj: Cleanup dead code

[Simon Marchi]

On 2016-02-07 07:56, Joel Brobecker wrote:
> FTR: There is actually no real agreement on this, and because each
> approach has its advantages, short of a royal edict, we'll probably
> continue accepting both forms.

Ah ok, I though it was agreed.

>> gdb/ChangeLog:
>> 
>> 	* varobj.h (varobj_delete): Remove dellist parameter, update and
>> 	move documentation here.
>> 	* varobj.c (struct cpstack, cppush, cppop): Remove.
>> 	(delete_variable): Remove resultp (first) parameter.
>> 	(delete_variable_1): Likewise.
>> 	(varobj_delete): Remove dellist parameter and unused code.
>> 	(update_dynamic_varobj_children): Adjust varobj_delete call.
>> 	(update_type_if_necessary): Likewise.
>> 	(varobj_set_visualizer): Likewise.
>> 	(varobj_update): Likewise.
>> 	(value_of_root): Likewise.
>> 	(varobj_invalidate_iter): Likewise.
>> 	* mi/mi-cmd-var.c (mi_cmd_var_delete): Likewise.
> 
> Looks good to me.
> 
> Thanks for doing that cleanup.

Pushed, thanks!