[PATCH] new macro: talloc_ctxdup

[Andrew Bartlett]

On Tue, 2012-05-01 at 07:40 -0400, Stephen Gallagher wrote:
> On Tue, 2012-05-01 at 21:26 +1000, Andrew Bartlett wrote:
> > On Tue, 2012-05-01 at 07:12 -0400, Stephen Gallagher wrote:
> ...
> > > This patch came out of a discussion that Pavel and I were having about
> > > copying memory out of a talloc pool into a new context. This seemed an
> > > expedient way to do it. Obviously as Andrew points out, you need to also
> > > be aware of whether the children of this context are also allocated from
> > > the pool.
> > > 
> > > Perhaps it would be better to design a talloc_steal_ext() that could be
> > > configured to recursively move child memory out of a pool, where
> > > appropriate.
> > 
> > I'm a little confused, as talloc_steal() already moves the context and
> > any child memory to the new parent.  What would your talloc_steal_ext()
> > do?
> 
> Actually, that's a false assumption. In the following construct:
> 
> TALLOC_CTX *tmp_ctx = talloc_pool(NULL, 65536);
> struct user *user = talloc(tmp_ctx, struct user);
> user->username = talloc_strdup(user, "user1");
> 
> Ok, so now we have 64k of memory allocated to a pool. We've taken some
> portion of it for use with user and its username.
> 
> Now:
> new_user = talloc_steal(mem_ctx, user);
> This reassigns the parent to the mem_ctx, but the memory is still
> allocated in the pool.
> 
> talloc_free(tmp_ctx); /* This doesn't do what you think it does */
> 
> The memory has its PARENTAGE as mem_ctx now, but the pool itself will
> not actually be freed until such time as the new_user is freed (and by
> extension new_user->username). So in the meantime, we're holding on to
> the entire 64k pool memory.

Indeed, this is the fundamental issue with talloc_pool().  If any memory
is kept longer than the desired pool life, this happens.  

I don't see a good solution however, unlike talloc_steal(), and more
like talloc_realloc() you would get a new pointer.  Nothing can
recursively fill in those pointers into the C struct, except by scanning
memory. 

For this reason, either we should decide that talloc_pool() isn't the
right solution for this kind of memory, or if you know that this memory
is needed outside the pool, that you never put it on the pool. 

talloc_pool() is a great performance hack, but like all performance
hacks, it works in it's optimal case, but has costs in other areas.  

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org