[PATCH]: talloc_pooled_object patchset
Jeremy Allison
jra at samba.org
Mon Aug 26 11:23:57 MDT 2013
On Mon, Aug 26, 2013 at 07:11:13PM +0200, Stefan (metze) Metzmacher wrote:
> Hi Jeremy,
> hi Volker,
>
> > On Thu, Aug 22, 2013 at 09:00:42PM -0700, Jeremy Allison wrote:
> >> Ok - here is a patchset I think is pushable to
> >> master if you agree :-).
> >>
> >> I took the liberty of adding your 'Signed-off-by'
> >> line as well as my 'Reviewed-by' tag.
> >>
> >> Turns out the only 3 changes needed to make this
> >> patchset valgrind-clean from my posted changes
> >> were the following :
> >
> > Thanks a lot for fixing those! Before pushing, I would like
> > metze to also give his blessing to the patchset. He will be
> > available on monday again, so I will ping him then.
>
> 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.
Comment on one of your TODO comments inside..
> From 4b475b176d6efb8b8e62d61786806bbea021b57c Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 9 May 2013 13:30:52 +0200
> Subject: [PATCH 06/12] TODO-SQUASH: talloc: Allow nested pools
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> Reviewed-by: Jeremy Allison <jra at samba.org>
>
> TODO: this removes TC_INVALIDATE_FULL_CHUNK(pool_tc); before free(pool);
> this should be readded...
> ---
> ....
> From 77f687b495f5b589cb305afdc379d17043705edc Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Mon, 26 Aug 2013 16:17:19 +0200
> Subject: [PATCH 07/12] sq talloc: Allow nested pools
>
> With this squash I'm happy with the commit...
>
> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> Reviewed-by: Stefan Metzmacher <metze at samba.org>
> ---
> lib/talloc/talloc.c | 1 +
> lib/talloc/talloc.h | 3 +--
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
> index 56d07b3..a112b3a 100644
> --- a/lib/talloc/talloc.c
> +++ b/lib/talloc/talloc.c
> @@ -829,6 +829,7 @@ static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
> if (pool_tc->flags & TALLOC_FLAG_POOLMEM) {
> _talloc_free_poolmem(pool_tc, location);
> } else {
> + TC_INVALIDATE_FULL_CHUNK(pool_tc);
> free(pool);
> }
> return;
That's funny. That's exactly the patch I originally had,
but then I realized that doing free(pool) implicitly does
exactly TC_INVALIDATE_FULL_CHUNK(pool_tc) so I removed it
entirely in the interests of efficiency.
We don't do an explitic invalidation on other code inside
talloc that calls free(), so I removed it as a redundency.
However if you think it makes the patchset clearer I'm
happy to re-add it - it just doesn't make any difference
to the valgrind mapping changes.
Cheers,
Jeremy.
More information about the samba-technical
mailing list