talloc_tos(), returning talloc_tos() memory and the talloc_stackframe order protection

[Stefan Metzmacher]

Am 22.06.2018 um 07:25 schrieb Andrew Bartlett via samba-technical:
> On Fri, 2018-06-15 at 14:29 +0200, Volker Lendecke via samba-technical
> wrote:
>> On Fri, Jun 15, 2018 at 02:07:26PM +0200, David Disseldorp wrote:
>>> I disagree. For short-lived heap-backed allocations, creating a new
>>> talloc child under a longer-term context and subsequently freeing it
>>> allows one to easily determine the lifecycle of the variable. This is
>>> not the case for talloc_tos() allocations without analysing the state of
>>> the talloc stackframe through the entire call chain - "quickly" is
>>> completely arbitrary.
>>
>> The problem is -- it's easy to forget to free the short-lived talloc
>> child. Because I've fallen into this trap more than once, I wrote
>> talloc_tos().
> 
> Isn't this why Rusty did the work to ensure that for developers, if you
> don't call talloc_free() on the talloc_stackframe() context we abort
> when we get to the next one?  
> 
> That seems to me to be a pretty solid protection.
> 
>>> A function returning memory allocated on the current stackframe without
>>> taking a talloc_tos() parameter is evil IMO, as it's completely unclear
>>> from the caller where the memory came from.
>>
>> Absolutely correct. You should NEVER return objects hung off
>> talloc_tos() from a function. This is evil IMHO too. If you have a
>> function that returns stuff, hand it a talloc context. This is vastly
>> different from temporary memory within a function where it does not
>> matter when exactly it's freed, immediately or a millisecond later.
> 
> Why do we have functions like create_conn_struct_tos() then?

This is a special case! The code we had before was much worse
it created connection_struct which did sometime chdir and forgot to
chdir back and call VFS_DISCONNECT when done.

create_conn_struct_[cwd]_tos() is now only usable in one code block
that starts with frame = talloc_stackframe() and ends with
TALLOC_FREE(frame). This implements a restriction similar to
become_root()/unbecome_root() which always be used as pair
wrapping a specific shortterm section.

It's intended that the caller only have a change to control the lifetime
of the connection_struct by using this pattern and hopefully don't
find a ways to skip the destructor from being called.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180622/fc6658a2/signature.sig>