[patch 3/4] varobj_list replacement, choice 3 of 3: Iterator

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

Iterator - so-called "safe" (keeping the next pointer) double link list:
+ It is algorithmically optimal complexity.
+ It keeps the root variables in the current order.
+ It has no testsuite changes / regressions.
+ More simple usage at the caller (no varobj_root_get_var).
+ Separates the interface and implementation parts.
+ Easy iterating the list being changed by varobj_delete + install_variable
  in the meantime.  One just keeps the "next" pointer before calling them.


Thanks,
Jan


gdb/
2009-07-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Replace public function varobj_list by all_root_varobjs iterator.
	* mi/mi-cmd-var.c (struct mi_cmd_var_update, mi_cmd_var_update_iter):
	New.
	(mi_cmd_var_update): Replace the varobj_list call by all_root_varobjs.
	Remove the variables rootlist, cr.  New variable data.
	* varobj.c (rootcount, varobj_list): Remove.
	(install_variable, uninstall_variable): Remove the rootcount updates.
	(all_root_varobjs): New function.
	(varobj_invalidate): Use the all_root_varobjs call.  Move the code to...
	(varobj_invalidate_iter): ... a new function.
	* varobj.h (varobj_list): Remove the prototype.
	(all_root_varobjs): New prototype.

--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -566,6 +566,41 @@ mi_cmd_var_assign (char *command, char **argv, int argc)
   ui_out_field_string (uiout, "value", varobj_get_value (var));
 }
 
+/* Type used for parameters passing to mi_cmd_var_update_iter.  */
+
+struct mi_cmd_var_update
+  {
+    int only_floating;
+    enum print_values print_values;
+  };
+
+/* Helper for mi_cmd_var_update - update each VAR.  */
+
+static void
+mi_cmd_var_update_iter (struct varobj *var, void *data_pointer)
+{
+  struct mi_cmd_var_update *data = data_pointer;
+  int thread_id, thread_stopped;
+
+  thread_id = varobj_get_thread_id (var);
+
+  if (thread_id == -1 && is_stopped (inferior_ptid))
+    thread_stopped = 1;
+  else
+    {
+      struct thread_info *tp = find_thread_id (thread_id);
+
+      if (tp)
+	thread_stopped = is_stopped (tp->ptid);
+      else
+	thread_stopped = 1;
+    }
+
+  if (thread_stopped)
+    if (!data->only_floating || varobj_floating_p (var))
+      varobj_update_one (var, data->print_values, 0 /* implicit */);
+}
+
 void
 mi_cmd_var_update (char *command, char **argv, int argc)
 {
@@ -596,31 +631,16 @@ mi_cmd_var_update (char *command, char **argv, int argc)
 
   if ((*name == '*' || *name == '@') && (*(name + 1) == '\0'))
     {
-      struct varobj **rootlist, **cr;
+      struct mi_cmd_var_update data;
 
-      varobj_list (&rootlist);
-      make_cleanup (xfree, rootlist);
+      data.only_floating = *name == '@';
+      data.print_values = print_values;
 
-      for (cr = rootlist; *cr != NULL; cr++)
-	{
-	  int thread_id = varobj_get_thread_id (*cr);
-	  int thread_stopped = 0;
+      /* varobj_update_one automatically updates all the children of VAROBJ.
+	 Therefore update each VAROBJ only once by iterating only the root
+	 VAROBJs.  */
 
-	  if (thread_id == -1 && is_stopped (inferior_ptid))
-	    thread_stopped = 1;
-	  else
-	    {
-	      struct thread_info *tp = find_thread_id (thread_id);
-	      if (tp)
-		thread_stopped = is_stopped (tp->ptid);
-	      else
-		thread_stopped = 1;
-	    }
-
-	  if (thread_stopped)
-	    if (*name == '*' || varobj_floating_p (*cr))
-	      varobj_update_one (*cr, print_values, 0 /* implicit */);
-	}
+      all_root_varobjs (mi_cmd_var_update_iter, &data);
     }
   else
     {
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -424,7 +424,6 @@ static int format_code[] = { 0, 't', 'd', 'x', 'o' };
 
 /* Header of the list of root variable objects */
 static struct varobj_root *rootlist;
-static int rootcount = 0;	/* number of root varobjs in the list */
 
 /* Prime number indicating the number of buckets in the hash table */
 /* A prime large enough to avoid too many colisions */
@@ -1169,37 +1168,6 @@ varobj_set_value (struct varobj *var, char *expression)
   return 1;
 }
 
-/* Returns a malloc'ed list with all root variable objects */
-int
-varobj_list (struct varobj ***varlist)
-{
-  struct varobj **cv;
-  struct varobj_root *croot;
-  int mycount = rootcount;
-
-  /* Alloc (rootcount + 1) entries for the result */
-  *varlist = xmalloc ((rootcount + 1) * sizeof (struct varobj *));
-
-  cv = *varlist;
-  croot = rootlist;
-  while ((croot != NULL) && (mycount > 0))
-    {
-      *cv = croot->rootvar;
-      mycount--;
-      cv++;
-      croot = croot->next;
-    }
-  /* Mark the end of the list */
-  *cv = NULL;
-
-  if (mycount || (croot != NULL))
-    warning
-      ("varobj_list: assertion failed - wrong tally of root vars (%d:%d)",
-       rootcount, mycount);
-
-  return rootcount;
-}
-
 /* Assign a new value to a variable object.  If INITIAL is non-zero,
    this is the first assignement after the variable object was just
    created, or changed type.  In that case, just assign the value 
@@ -1746,7 +1714,6 @@ install_variable (struct varobj *var)
       else
 	var->root->next = rootlist;
       rootlist = var->root;
-      rootcount++;
     }
 
   return 1;			/* OK */
@@ -1823,7 +1790,6 @@ uninstall_variable (struct varobj *var)
 	  else
 	    prer->next = cr->next;
 	}
-      rootcount--;
     }
 
 }
@@ -3206,6 +3172,24 @@ java_value_of_variable (struct varobj *var, enum varobj_display_formats format)
 {
   return cplus_value_of_variable (var, format);
 }
+
+/* Iterate all the existing _root_ VAROBJs and call the FUNC callback for them
+   with an arbitrary caller supplied DATA pointer.  */
+
+void
+all_root_varobjs (void (*func) (struct varobj *var, void *data), void *data)
+{
+  struct varobj_root *var_root, *var_root_next;
+
+  /* Iterate "safely" - handle if the callee deletes its passed VAROBJ.  */
+
+  for (var_root = rootlist; var_root != NULL; var_root = var_root_next)
+    {
+      var_root_next = var_root->next;
+
+      (*func) (var_root->rootvar, data);
+    }
+}
 
 extern void _initialize_varobj (void);
 void
@@ -3226,48 +3210,45 @@ When non-zero, varobj debugging is enabled."),
 			    &setlist, &showlist);
 }
 
-/* Invalidate the varobjs that are tied to locals and re-create the ones that
-   are defined on globals.
-   Invalidated varobjs will be always printed in_scope="invalid".  */
+/* Invalidate varobj VAR if it is tied to locals and re-create it if it is
+   defined on globals.  It is a helper for varobj_invalidate.  */
 
-void 
-varobj_invalidate (void)
+static void
+varobj_invalidate_iter (struct varobj *var, void *unused)
 {
-  struct varobj **all_rootvarobj;
-  struct varobj **varp;
+  /* Floating varobjs are reparsed on each stop, so we don't care if the
+     presently parsed expression refers to something that's gone.  */
+  if (var->root->floating)
+    return;
 
-  if (varobj_list (&all_rootvarobj) > 0)
+  /* global var must be re-evaluated.  */     
+  if (var->root->valid_block == NULL)
     {
-      for (varp = all_rootvarobj; *varp != NULL; varp++)
-	{
-	  /* Floating varobjs are reparsed on each stop, so we don't care if
-	     the presently parsed expression refers to something that's gone.
-	     */
-	  if ((*varp)->root->floating)
-	    continue;
+      struct varobj *tmp_var;
 
-	  /* global var must be re-evaluated.  */     
-	  if ((*varp)->root->valid_block == NULL)
-	    {
-	      struct varobj *tmp_var;
-
-	      /* Try to create a varobj with same expression.  If we succeed
-		 replace the old varobj, otherwise invalidate it.  */
-	      tmp_var = varobj_create (NULL, (*varp)->name, (CORE_ADDR) 0,
-				       USE_CURRENT_FRAME);
-	      if (tmp_var != NULL) 
-		{ 
-		  tmp_var->obj_name = xstrdup ((*varp)->obj_name);
-		  varobj_delete (*varp, NULL, 0);
-		  install_variable (tmp_var);
-		}
-	      else
-		(*varp)->root->is_valid = 0;
-	    }
-	  else /* locals must be invalidated.  */
-	    (*varp)->root->is_valid = 0;
+      /* Try to create a varobj with same expression.  If we succeed
+	 replace the old varobj, otherwise invalidate it.  */
+      tmp_var = varobj_create (NULL, var->name, (CORE_ADDR) 0,
+			       USE_CURRENT_FRAME);
+      if (tmp_var != NULL) 
+	{ 
+	  tmp_var->obj_name = xstrdup (var->obj_name);
+	  varobj_delete (var, NULL, 0);
+	  install_variable (tmp_var);
 	}
+      else
+	var->root->is_valid = 0;
     }
-  xfree (all_rootvarobj);
-  return;
+  else /* locals must be invalidated.  */
+    var->root->is_valid = 0;
+}
+
+/* Invalidate the varobjs that are tied to locals and re-create the ones that
+   are defined on globals.
+   Invalidated varobjs will be always printed in_scope="invalid".  */
+
+void 
+varobj_invalidate (void)
+{
+  all_root_varobjs (varobj_invalidate_iter, NULL);
 }
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -137,7 +137,8 @@ extern char *varobj_get_value (struct varobj *var);
 
 extern int varobj_set_value (struct varobj *var, char *expression);
 
-extern int varobj_list (struct varobj ***rootlist);
+extern void all_root_varobjs (void (*func) (struct varobj *var, void *data),
+			      void *data);
 
 extern VEC(varobj_update_result) *varobj_update (struct varobj **varp, 
 						 int explicit);