This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/6] Mention which return values need to be freed in lang_varobj_ops
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: <gdb-patches at sourceware dot org>
- Date: Fri, 30 Jan 2015 11:41:48 -0500
- Subject: Re: [PATCH 2/6] Mention which return values need to be freed in lang_varobj_ops
- Authentication-results: sourceware.org; auth=none
- References: <1422559716-5480-1-git-send-email-simon dot marchi at ericsson dot com> <1422559716-5480-2-git-send-email-simon dot marchi at ericsson dot com> <20150130032848 dot GK5193 at adacore dot 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?