This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] MI: Free values when updating


On Wednesday 24 January 2007 00:19, Nick Roberts wrote:

>  > > I think if you also remove the (3) calls to release_value in
>  > > c_value_of_child and cplus_value_of_child this is equivalent to my change
>  > > (and more tidy).
>  > 
>  > No, those are different.  They come from things like the call to
>  > gdb_value_ind in c_describe_child.  That creates a new value, which is
>  > returned to the caller (the MI front end, to be printed and later
>  > released).  It's the ones in c_value_of_root which matter, because we
>  > save them in the varobj.
> 
> In varobj_update:
> 
> 	  new = value_of_child (v->parent, v->index);
> 	  if (install_new_value (v, new, 0 /* type not changed */))
> 
> In create_child:
> 
>   value = value_of_child (parent, index);
>   ...
>   install_new_value (child, value, 1);
> 
> Now install_new_value calls release_value on new, it's not neeeded in
> *_value_of_child.  

I think that's right. All calls to release_value except in install_new_value can be
removed I believe, and I'll post a patch to that effect tomorrow.

> I've tried this change on the MI testsuite and see no fails 
> and it has about the same time improvement that my patch had.  If this is not
> the right patch then the testsuite is lacking the appropriate test.  Can you
> create a test where this patch fails?

You cannot. For non-reference types, release_value gets called twice, which is
benign now. I'll try adding assert in release_value, though.

For reference and array types, extra release_value gives you a memleak, and 
there's nothing that will cause a test to fail due to that.

- Volodya


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]