Re: [PATCH 2/6] Mention which return values need to be freed in lang_varobj_ops

[Simon Marchi <simon.marchi@ericsson.com>]

On 15-01-29 10:28 PM, Joel Brobecker wrote:
>> gdb/ChangeLog:
>>
>> 	* varobj.h (lang_varobj_ops): Mention which return values need
>> 	to be freed.
> 
> Thanks for doing that! One question...
> 
>> -  /* The ``struct value *'' of the INDEX'th child of PARENT.  */
>> +  /* The ``struct value *'' of the INDEX'th child of PARENT.  The returned
>> +     value must be freed by the caller.  */
>>    struct value *(*value_of_child) (struct varobj *parent, int index);
> 
> I'm really surprised by this. For memory management, the struct value
> objects are put on a chain. So, you wouldn't delete the value returned,
> but you would instead use "value_mark/value_free_to_mark". The top-level
> command loop takes a mark at the beginning of the command, and uses it
> to free any un-freed value after the command completes.
> 
> But maybe you saw something that contradicts my understanding?

After looking more closely, I think you are right. Originally, I saw that
install_new_value called value_free on the old value and jumped to the
conclusion. Actually, value_free is more like a "value_decref", which
frees the variable if the reference count drops to 0. The call to
value_free just matches the value_incref that was also done in
install_new_value when we installed the value. So just calling
value_of_child doesn't mean that you have to call value_free.

Thanks for the explanation, I didn't know about the memory management of
values. I'll remove the comment change for value_of_child. Is the rest of
the patch ok?