talloc_tos() vs talloc_stackframe()
abartlet at samba.org
Mon Jul 2 04:25:00 UTC 2018
On Fri, 2018-06-22 at 07:46 +0200, Volker Lendecke wrote:
> On Fri, Jun 22, 2018 at 03:35:22PM +1200, Andrew Bartlett wrote:
> > On Fri, 2018-06-15 at 08:36 +0200, Volker Lendecke via samba-technical
> > wrote:
> > > 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.
> Just to clarify: We used to have a pattern where we handed down a
> talloc context for temporary use within a function. This is what I
> object to and want to see it replaced by talloc_tos().
I would prefer a new talloc_stackframe(), but yes.
> Once a function
> returns allocated memory, we must always pass down the talloc context
> to put this on.
> We must never return memory for later use on top of
I strongly agree. Can someone please have a look at
source3/smbd/posix_acls.c? Every function within that file seems to
assume that talloc_tos() memory lasts until some top-level caller, and
returns memory on that context.
In related news, we did find the sysvolreset performance issue, it is
actually the uid_entry_in_group() call hidden in there, re-building the
user's SID list (tokenGroups) for every ACL just to see if the user
might be in a group!
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
More information about the samba-technical