Re: [PATCH 3/7] Remove field value_of_root

[Yao Qi <yao at codesourcery dot com>]

On 10/01/2013 06:16 PM, Joel Brobecker wrote:
> I think it's a step in the right direction, as I don't see how
> we would want a language-specific value-of-root. So, LGTM, but
> could you also add an empty line after the description of function
> check_scope? Also, Could you mention that "c_value_of_root" was
> moved earlier as well (ie: "Move earlier and renamed to...").

Add this sentence into changelog entry.

> > >+/* If frame associated with VAR can be found, switch
> > >+   to it and return 1.  Otherwise, return 0.  */
> > >+static int
> > >+check_scope (struct varobj *var)

Fixed.

Patch below is committed.

--
Yao (éå)

gdb:

2013-10-01  Yao Qi  <yao@codesourcery.com>

	* varobj.c (c_value_of_root): Remove declaration.
	(cplus_value_of_root, java_value_of_root): Likewise.
	(ada_value_of_root): Likewise.
	(struct language_specific) <value_of_root>: Remove.
	(languages): Update initialization.
	(check_scope): Move earlier.
	(c_value_of_root): Move earlier and rename to ...
	(value_of_root_1): ... this.
	(value_of_root): Caller update.
	(cplus_value_of_root, java_value_of_root): Remove.
	(ada_value_of_root): Remove.
---
 gdb/varobj.c |  192 
++++++++++++++++++++++++---------------------------------
 1 files changed, 81 insertions(+), 111 deletions(-)

diff --git a/gdb/varobj.c b/gdb/varobj.c
index bcf62dc..007574f 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -321,8 +321,6 @@ static char *c_name_of_child (struct varobj *parent, 
int index);

 static char *c_path_expr_of_child (struct varobj *child);

-static struct value *c_value_of_root (struct varobj **var_handle);
-
 static struct value *c_value_of_child (struct varobj *parent, int index);

 static struct type *c_type_of_child (struct varobj *parent, int index);
@@ -342,8 +340,6 @@ static char *cplus_name_of_child (struct varobj 
*parent, int index);

 static char *cplus_path_expr_of_child (struct varobj *child);

-static struct value *cplus_value_of_root (struct varobj **var_handle);
-
 static struct value *cplus_value_of_child (struct varobj *parent, int 
index);

 static struct type *cplus_type_of_child (struct varobj *parent, int 
index);
@@ -361,8 +357,6 @@ static char *java_name_of_child (struct varobj 
*parent, int index);

 static char *java_path_expr_of_child (struct varobj *child);

-static struct value *java_value_of_root (struct varobj **var_handle);
-
 static struct value *java_value_of_child (struct varobj *parent, int 
index);

 static struct type *java_type_of_child (struct varobj *parent, int index);
@@ -380,8 +374,6 @@ static char *ada_name_of_child (struct varobj 
*parent, int index);

 static char *ada_path_expr_of_child (struct varobj *child);

-static struct value *ada_value_of_root (struct varobj **var_handle);
-
 static struct value *ada_value_of_child (struct varobj *parent, int 
index);

 static struct type *ada_type_of_child (struct varobj *parent, int index);
@@ -411,9 +403,6 @@ struct language_specific
      obtain that has some parent.  */
   char *(*path_expr_of_child) (struct varobj * child);

-  /* The ``struct value *'' of the root variable ROOT.  */
-  struct value *(*value_of_root) (struct varobj ** root_handle);
-
   /* The ``struct value *'' of the INDEX'th child of PARENT.  */
   struct value *(*value_of_child) (struct varobj * parent, int index);

@@ -458,7 +447,6 @@ static struct language_specific languages[vlang_end] = {
    c_name_of_variable,
    c_name_of_child,
    c_path_expr_of_child,
-   c_value_of_root,
    c_value_of_child,
    c_type_of_child,
    c_value_of_variable,
@@ -471,7 +459,6 @@ static struct language_specific languages[vlang_end] = {
    cplus_name_of_variable,
    cplus_name_of_child,
    cplus_path_expr_of_child,
-   cplus_value_of_root,
    cplus_value_of_child,
    cplus_type_of_child,
    cplus_value_of_variable,
@@ -484,7 +471,6 @@ static struct language_specific languages[vlang_end] = {
    java_name_of_variable,
    java_name_of_child,
    java_path_expr_of_child,
-   java_value_of_root,
    java_value_of_child,
    java_type_of_child,
    java_value_of_variable,
@@ -496,7 +482,6 @@ static struct language_specific languages[vlang_end] = {
    ada_name_of_variable,
    ada_name_of_child,
    ada_path_expr_of_child,
-   ada_value_of_root,
    ada_value_of_child,
    ada_type_of_child,
    ada_value_of_variable,
@@ -2710,6 +2695,86 @@ name_of_child (struct varobj *var, int index)
   return (*var->root->lang->name_of_child) (var, index);
 }

+/* If frame associated with VAR can be found, switch
+   to it and return 1.  Otherwise, return 0.  */
+
+static int
+check_scope (struct varobj *var)
+{
+  struct frame_info *fi;
+  int scope;
+
+  fi = frame_find_by_id (var->root->frame);
+  scope = fi != NULL;
+
+  if (fi)
+    {
+      CORE_ADDR pc = get_frame_pc (fi);
+
+      if (pc <  BLOCK_START (var->root->valid_block) ||
+	  pc >= BLOCK_END (var->root->valid_block))
+	scope = 0;
+      else
+	select_frame (fi);
+    }
+  return scope;
+}
+
+/* Helper function to value_of_root.  */
+
+static struct value *
+value_of_root_1 (struct varobj **var_handle)
+{
+  struct value *new_val = NULL;
+  struct varobj *var = *var_handle;
+  int within_scope = 0;
+  struct cleanup *back_to;
+								
+  /*  Only root variables can be updated...  */
+  if (!is_root_p (var))
+    /* Not a root var.  */
+    return NULL;
+
+  back_to = make_cleanup_restore_current_thread ();
+
+  /* Determine whether the variable is still around.  */
+  if (var->root->valid_block == NULL || var->root->floating)
+    within_scope = 1;
+  else if (var->root->thread_id == 0)
+    {
+      /* The program was single-threaded when the variable object was
+	 created.  Technically, it's possible that the program became
+	 multi-threaded since then, but we don't support such
+	 scenario yet.  */
+      within_scope = check_scope (var);	
+    }
+  else
+    {
+      ptid_t ptid = thread_id_to_pid (var->root->thread_id);
+      if (in_thread_list (ptid))
+	{
+	  switch_to_thread (ptid);
+	  within_scope = check_scope (var);
+	}
+    }
+
+  if (within_scope)
+    {
+      volatile struct gdb_exception except;
+
+      /* We need to catch errors here, because if evaluate
+         expression fails we want to just return NULL.  */
+      TRY_CATCH (except, RETURN_MASK_ERROR)
+	{
+	  new_val = evaluate_expression (var->root->exp);
+	}
+    }
+
+  do_cleanups (back_to);
+
+  return new_val;
+}
+
 /* What is the ``struct value *'' of the root variable VAR?
    For floating variable object, evaluation can get us a value
    of different type from what is stored in varobj already.  In
@@ -2787,7 +2852,7 @@ value_of_root (struct varobj **var_handle, int 
*type_changed)
   {
     struct value *value;

-    value = (*var->root->lang->value_of_root) (var_handle);
+    value = value_of_root_1 (var_handle);
     if (var->value == NULL || value == NULL)
       {
 	/* For root varobj-s, a NULL value indicates a scoping issue.
@@ -3382,83 +3447,6 @@ c_path_expr_of_child (struct varobj *child)
   return child->path_expr;
 }

-/* If frame associated with VAR can be found, switch
-   to it and return 1.  Otherwise, return 0.  */
-static int
-check_scope (struct varobj *var)
-{
-  struct frame_info *fi;
-  int scope;
-
-  fi = frame_find_by_id (var->root->frame);
-  scope = fi != NULL;
-
-  if (fi)
-    {
-      CORE_ADDR pc = get_frame_pc (fi);
-
-      if (pc <  BLOCK_START (var->root->valid_block) ||
-	  pc >= BLOCK_END (var->root->valid_block))
-	scope = 0;
-      else
-	select_frame (fi);
-    }
-  return scope;
-}
-
-static struct value *
-c_value_of_root (struct varobj **var_handle)
-{
-  struct value *new_val = NULL;
-  struct varobj *var = *var_handle;
-  int within_scope = 0;
-  struct cleanup *back_to;
-								
-  /*  Only root variables can be updated...  */
-  if (!is_root_p (var))
-    /* Not a root var.  */
-    return NULL;
-
-  back_to = make_cleanup_restore_current_thread ();
-
-  /* Determine whether the variable is still around.  */
-  if (var->root->valid_block == NULL || var->root->floating)
-    within_scope = 1;
-  else if (var->root->thread_id == 0)
-    {
-      /* The program was single-threaded when the variable object was
-	 created.  Technically, it's possible that the program became
-	 multi-threaded since then, but we don't support such
-	 scenario yet.  */
-      within_scope = check_scope (var);	
-    }
-  else
-    {
-      ptid_t ptid = thread_id_to_pid (var->root->thread_id);
-      if (in_thread_list (ptid))
-	{
-	  switch_to_thread (ptid);
-	  within_scope = check_scope (var);
-	}
-    }
-
-  if (within_scope)
-    {
-      volatile struct gdb_exception except;
-
-      /* We need to catch errors here, because if evaluate
-         expression fails we want to just return NULL.  */
-      TRY_CATCH (except, RETURN_MASK_ERROR)
-	{
-	  new_val = evaluate_expression (var->root->exp);
-	}
-    }
-
-  do_cleanups (back_to);
-
-  return new_val;
-}
-
 static struct value *
 c_value_of_child (struct varobj *parent, int index)
 {
@@ -3893,12 +3881,6 @@ cplus_path_expr_of_child (struct varobj *child)
 }

 static struct value *
-cplus_value_of_root (struct varobj **var_handle)
-{
-  return c_value_of_root (var_handle);
-}
-
-static struct value *
 cplus_value_of_child (struct varobj *parent, int index)
 {
   struct value *value = NULL;
@@ -3983,12 +3965,6 @@ java_path_expr_of_child (struct varobj *child)
 }

 static struct value *
-java_value_of_root (struct varobj **var_handle)
-{
-  return cplus_value_of_root (var_handle);
-}
-
-static struct value *
 java_value_of_child (struct varobj *parent, int index)
 {
   return cplus_value_of_child (parent, index);
@@ -4041,12 +4017,6 @@ ada_path_expr_of_child (struct varobj *child)
 }

 static struct value *
-ada_value_of_root (struct varobj **var_handle)
-{
-  return c_value_of_root (var_handle);
-}
-
-static struct value *
 ada_value_of_child (struct varobj *parent, int index)
 {
   return ada_varobj_get_value_of_child (parent->value, parent->type,
--
1.7.7.6