[PATCH]: talloc_pooled_object patchset

[Jeremy Allison]

On Mon, Aug 26, 2013 at 07:11:13PM +0200, Stefan (metze) Metzmacher wrote:
> 
> I reviewed the patches, I'm almost happy with them.
> 
> I just found a few minor things we need to discuss/fix.
> 
> The attached patches have TODO comments in their commit message.

One more thing I'm working on (inline below).

> From 36cd67516ff9854293f0bf31b158416cd40c9d95 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Sun, 5 May 2013 17:30:34 +0200
> Subject: [PATCH 03/12] TODO-FIX: talloc: Introduce __talloc_with_prefix
> 
> This will allow to exchange the extra talloc pool header with the
> talloc_chunk structure
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> Reviewed-by: Jeremy Allison <jra at samba.org>
> 
> TODO: l->cur_size += total_len; doesn't match the talloc_memlimit_check()
> arguments, as we don't record the prefix_size somewhere, we can't decrement
> it correctly on free, so we should ignore the size?
> OR: maybe even better include prefix_len in all places,
> should we record the prefix_len?

The talloc_memlimit_check() code doesn't work correctly
with pools - even before this patchset.

As an example - creating a pool using talloc_pool()
internally uses __talloc(), which then uses the
talloc_memlimit accounting to add in the requested
pool size into l->cur_size.

However, allocating from inside the pool via
talloc_alloc_pool() also goes through the code
that adds in the newly allocated size into
l->cur_size - even though no new malloc was
done. If you like I can fix this up and then
add the new patchset rebased on top.

> TODO: don't we need integer wrap protection?

Not with the :

        if (unlikely(size >= MAX_TALLOC_SIZE)) {
                return NULL;
        }

check before the additions.

Jeremy.