talloc_tos() vs talloc_stackframe()
abartlet at samba.org
Fri Jun 22 03:35:22 UTC 2018
On Fri, 2018-06-15 at 08:36 +0200, Volker Lendecke via samba-technical
> On Fri, Jun 15, 2018 at 12:10:50AM +0200, David Disseldorp via samba-technical wrote:
> > Hi Metze,
> > On Thu, 14 Jun 2018 20:54:26 +0200, Stefan Metzmacher wrote:
> > > For async code we always have the 'state' variable to use for temporary
> > > memory, so we don't need talloc_stackframes...
> > Indeed, we just need to encourage people to use it instead of the
> > talloc_tos() context, and begin migrating over helper functions :)
> Do you really want to pass down temporary talloc contexts everywhere?
> That's exactly what talloc_tos() was designed for.
Yes, we should. If memory is returned, it should be on a context.
The problem with silently allocating memory on talloc_tos() is 'how
long is this valid for?'.
At the time it is written, it might be valid quite a way up the C
stack, but then adding in a talloc_stackframe() at any point then
invalidates the assumptions.
As you know, and we have had 'words' about this before, I dislike
talloc_tos() for this reason, and because more broadly across the
codebase it is not universally true that there is an outer stackframe.
(While historically hesitant, I do use talloc_stackframe() where
appropriate in new code for talloc memory needed within a function).
I see that create_conn_struct_tos() was recently pushed though more of
the code, but I'm strongly of the view that this is less clear and the
wrong direction. It might be better than create_conn_struct() for
other reasons, but I really did think we were moving away from this
implicit memory allocation.
Likewise, the recent 'pysmbd: remove explicit talloc_stackframe() from
get_conn() and name it get_conn_tos()' just seems wrong to me.
On the flip side, the approach taken in the recent patch 'pysmbd: fix
some talloc_stackframe() memory leaks and clean up the frame hierarchy
in make_simple_acl().' is the right way. I'm sorry for the original
bug, but I agree with the fix, provide a memory context and use it.
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
More information about the samba-technical