[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