Patchwork [3/6] Set varobj->path_expr in varobj_get_path_expr

login
register
mail settings
Submitter Simon Marchi
Date Jan. 29, 2015, 7:28 p.m.
Message ID <1422559716-5480-3-git-send-email-simon.marchi@ericsson.com>
Download mbox | patch
Permalink /patch/4846/
State New
Headers show

Comments

Simon Marchi - Jan. 29, 2015, 7:28 p.m.
It seems like different languages are doing this differently (e.g.
C and Ada). For C, var->path_expr is set inside c_path_expr_of_child.
The next time the value is requested, is it therefore not recomputed.
Ada does not set this field, but just returns the value. Since the field
is never set, the value is recomputed every time it is requested.

This patch makes it so that path_expr_of_child's only job is to compute
the path expression, not save/cache the value. The field is set by the
varobj common code.

gdb/ChangeLog:

	* varobj.c (varobj_get_path_expr): Set var->path_expr.
	* c-varobj.c (c_path_expr_of_child): Set local var instead of
	child->path_expr.
	(cplus_path_expr_of_child): Same.
---
 gdb/c-varobj.c | 12 ++++++++----
 gdb/varobj.c   |  9 +++++----
 2 files changed, 13 insertions(+), 8 deletions(-)
Joel Brobecker - Jan. 30, 2015, 3:39 a.m.
> It seems like different languages are doing this differently (e.g.
> C and Ada). For C, var->path_expr is set inside c_path_expr_of_child.
> The next time the value is requested, is it therefore not recomputed.
> Ada does not set this field, but just returns the value. Since the field
> is never set, the value is recomputed every time it is requested.
> 
> This patch makes it so that path_expr_of_child's only job is to compute
> the path expression, not save/cache the value. The field is set by the
> varobj common code.

Nice little cleanup, IMO. Thanks for doing it.

> gdb/ChangeLog:
> 
> 	* varobj.c (varobj_get_path_expr): Set var->path_expr.
> 	* c-varobj.c (c_path_expr_of_child): Set local var instead of
> 	child->path_expr.
> 	(cplus_path_expr_of_child): Same.

LGTM. Thank you!
Simon Marchi - Jan. 30, 2015, 7:46 p.m.
On 15-01-29 10:39 PM, Joel Brobecker wrote:
>> It seems like different languages are doing this differently (e.g.
>> C and Ada). For C, var->path_expr is set inside c_path_expr_of_child.
>> The next time the value is requested, is it therefore not recomputed.
>> Ada does not set this field, but just returns the value. Since the field
>> is never set, the value is recomputed every time it is requested.
>>
>> This patch makes it so that path_expr_of_child's only job is to compute
>> the path expression, not save/cache the value. The field is set by the
>> varobj common code.
> 
> Nice little cleanup, IMO. Thanks for doing it.
> 
>> gdb/ChangeLog:
>>
>> 	* varobj.c (varobj_get_path_expr): Set var->path_expr.
>> 	* c-varobj.c (c_path_expr_of_child): Set local var instead of
>> 	child->path_expr.
>> 	(cplus_path_expr_of_child): Same.
> 
> LGTM. Thank you!

Pushed, thanks!

Patch

diff --git a/gdb/c-varobj.c b/gdb/c-varobj.c
index 1db0957..bd0e5fb 100644
--- a/gdb/c-varobj.c
+++ b/gdb/c-varobj.c
@@ -433,9 +433,11 @@  c_name_of_child (struct varobj *parent, int index)
 static char *
 c_path_expr_of_child (struct varobj *child)
 {
+  char *path_expr;
+
   c_describe_child (child->parent, child->index, NULL, NULL, NULL, 
-		    &child->path_expr);
-  return child->path_expr;
+		    &path_expr);
+  return path_expr;
 }
 
 static struct value *
@@ -906,9 +908,11 @@  cplus_name_of_child (struct varobj *parent, int index)
 static char *
 cplus_path_expr_of_child (struct varobj *child)
 {
+  char *path_expr;
+
   cplus_describe_child (child->parent, child->index, NULL, NULL, NULL, 
-			&child->path_expr);
-  return child->path_expr;
+			&path_expr);
+  return path_expr;
 }
 
 static struct value *
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 6c9257d..28d388e 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1034,16 +1034,17 @@  varobj_get_path_expr_parent (struct varobj *var)
 char *
 varobj_get_path_expr (struct varobj *var)
 {
-  if (var->path_expr != NULL)
-    return var->path_expr;
-  else 
+  if (var->path_expr == NULL)
     {
       /* For root varobjs, we initialize path_expr
 	 when creating varobj, so here it should be
 	 child varobj.  */
       gdb_assert (!is_root_p (var));
-      return (*var->root->lang_ops->path_expr_of_child) (var);
+
+      var->path_expr = (*var->root->lang_ops->path_expr_of_child) (var);
     }
+
+  return var->path_expr;
 }
 
 const struct language_defn *