[PATCH]: talloc_pooled_object patchset

[Stefan (metze) Metzmacher]

Hi Jeremy,

>>> 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.

TC_INVALIDATE_FULL_CHUNK() does
TC_INVALIDATE_FULL_VALGRIND_CHUNK, but also
TC_INVALIDATE_FULL_FILL_CHUNK.

For TC_INVALIDATE_FULL_VALGRIND_CHUNK you're right that would not be needed.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130826/2dba83f9/attachment.pgp>