[mi] kill release_value calls

[mi] kill release_value calls

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]


This patch removes all calls to release_value in varobj.c, except
for one. We only need to call release_value when we're storing
a value for indefinite future use, and install_new_value is the
only such place.  The extra release_value calls either 
are time-consuming no-ops, or lead to memory leaks, 
depending on the value type.

OK?

- Volodya

	* varobj.c (c_value_of_root, c_value_of_child)
	(cplus_value_of_child): Don't call release_value.


[-- Attachment #2: release_value__gdb_mainline.diff --]
[-- Type: text/x-diff, Size: 1380 bytes --]

--- gdb/varobj.c	(/mirrors/gdb_mainline)	(revision 3220)
+++ gdb/varobj.c	(/patches/gdb/release_value/gdb_mainline)	(revision 3220)
@@ -1985,13 +1985,8 @@ c_value_of_root (struct varobj **var_han
   if (within_scope)
     {
       /* We need to catch errors here, because if evaluate
-         expression fails we just want to make val->error = 1 and
-         go on */
-      if (gdb_evaluate_expression (var->root->exp, &new_val))
-	{
-	  release_value (new_val);
-	}
-
+         expression fails we want to just return NULL.  */
+      gdb_evaluate_expression (var->root->exp, &new_val);
       return new_val;
     }
 
@@ -2003,8 +1998,6 @@ c_value_of_child (struct varobj *parent,
 {
   struct value *value = NULL;
   c_describe_child (parent, index, NULL, &value, NULL);
-  if (value != NULL)
-    release_value (value);
 
   return value;
 }
@@ -2338,9 +2331,6 @@ cplus_value_of_child (struct varobj *par
 	  name = name_of_child (parent, index);
 	  gdb_value_struct_elt (NULL, &value, &temp, NULL, name, NULL,
 				"cplus_structure");
-	  if (value != NULL)
-	    release_value (value);
-
 	  xfree (name);
 	}
       else if (index >= TYPE_N_BASECLASSES (type))
@@ -2368,7 +2358,6 @@ cplus_value_of_child (struct varobj *par
 	      if (temp != NULL)
 		{
 		  value = value_cast (TYPE_FIELD_TYPE (type, index), temp);
-		  release_value (value);
 		}
 	      else
 		{

Re: [mi] kill release_value calls

[Daniel Jacobowitz]

On Wed, Jan 24, 2007 at 10:33:30PM +0300, Vladimir Prus wrote:
> 
> This patch removes all calls to release_value in varobj.c, except
> for one. We only need to call release_value when we're storing
> a value for indefinite future use, and install_new_value is the
> only such place.  The extra release_value calls either 
> are time-consuming no-ops, or lead to memory leaks, 
> depending on the value type.
> 
> OK?

OK.  My apologies to Nick - I'd told him this was wrong, but I was
mistaken.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [mi] kill release_value calls

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 663 bytes --]

Daniel Jacobowitz wrote:

> On Wed, Jan 24, 2007 at 10:33:30PM +0300, Vladimir Prus wrote:
>> 
>> This patch removes all calls to release_value in varobj.c, except
>> for one. We only need to call release_value when we're storing
>> a value for indefinite future use, and install_new_value is the
>> only such place.  The extra release_value calls either
>> are time-consuming no-ops, or lead to memory leaks,
>> depending on the value type.
>> 
>> OK?
> 
> OK.  My apologies to Nick - I'd told him this was wrong, but I was
> mistaken.

Here's what I've checked in -- I had to adjust the patch
for the changes I've checked to mainline earlier today.

- Volodya


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: release_value_as_committed.diff --]
[-- Type: text/x-diff; name="release_value_as_committed.diff", Size: 1754 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.8137
diff -u -p -r1.8137 ChangeLog
--- ChangeLog	24 Jan 2007 13:37:05 -0000	1.8137
+++ ChangeLog	24 Jan 2007 19:53:01 -0000
@@ -1,3 +1,8 @@
+2007-01-24  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* varobj.c (c_value_of_root, c_value_of_child)
+	(cplus_describe_child): Don't call release_value.
+
 2007-01-24  Thiemo Seufer  <ths@mips.com>
 
 	* mips-linux-tdep.c (mips_linux_n64_rt_sigframe): Fix struct
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.81
diff -u -p -r1.81 varobj.c
--- varobj.c	24 Jan 2007 11:08:47 -0000	1.81
+++ varobj.c	24 Jan 2007 19:53:02 -0000
@@ -2020,13 +2020,8 @@ c_value_of_root (struct varobj **var_han
   if (within_scope)
     {
       /* We need to catch errors here, because if evaluate
-         expression fails we just want to make val->error = 1 and
-         go on */
-      if (gdb_evaluate_expression (var->root->exp, &new_val))
-	{
-	  release_value (new_val);
-	}
-
+         expression fails we want to just return NULL.  */
+      gdb_evaluate_expression (var->root->exp, &new_val);
       return new_val;
     }
 
@@ -2038,8 +2033,6 @@ c_value_of_child (struct varobj *parent,
 {
   struct value *value = NULL;
   c_describe_child (parent, index, NULL, &value, NULL);
-  if (value != NULL)
-    release_value (value);
 
   return value;
 }
@@ -2304,7 +2297,6 @@ cplus_describe_child (struct varobj *par
 	  if (cvalue && value)
 	    {
 	      *cvalue = value_cast (TYPE_FIELD_TYPE (type, index), value);
-	      release_value (*cvalue);
 	    }
 
 	  if (ctype)