On (21/09/15 10:40), Pavel Březina wrote:
On 09/20/2015 07:58 PM, Pavel Reichl wrote:
>
>
>On 09/04/2015 01:56 PM, Jakub Hrozek wrote:
>>On Fri, Sep 04, 2015 at 01:09:36PM +0200, Pavel Reichl wrote:
>>>Hello,
>>>
>>>please see simple patch set fixing minor memory leaks of providers.
>>>I'm not aware of any user hitting those currently.
>>>
>>>Thanks!
>>
>>> From ef62f0245cde314fd11b1cd2584589e018ede050 Mon Sep 17 00:00:00 2001
>>>From: Pavel Reichl <preichl(a)redhat.com>
>>>Date: Fri, 4 Sep 2015 07:02:42 -0400
>>>Subject: [PATCH 1/4] AD: fix minor memory leak
>>>
>>>---
>>> src/providers/ad/ad_common.c | 7 ++++---
>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>>diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
>>>index
>>>130cdeb613aae3843f7453a478815daaae6aab77..aca550581708e6501c99f1c69e3c5ec9303d3b8c
>>>100644
>>>--- a/src/providers/ad/ad_common.c
>>>+++ b/src/providers/ad/ad_common.c
>>>@@ -658,7 +658,7 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct
>>>be_ctx *bectx,
>>> TALLOC_CTX *tmp_ctx;
>>> struct ad_service *service;
>>>
>>>- tmp_ctx = talloc_new(mem_ctx);
>>>+ tmp_ctx = talloc_new(NULL);
>>
>>I don't think we should do this kind of change, it's quite possible
>>we'll start using talloc pools soon which will make us migrate from
>>using NULL context.
>>
>>Just make sure tmp_ctx is freed as appropriate.
>>
>>I haven't reviewed the rest of the patches.
>>
>>> if (!tmp_ctx) return ENOMEM;
>>_______________________________________________
>>sssd-devel mailing list
>>sssd-devel(a)lists.fedorahosted.org
>>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>
>Bump. (I think that the first patch nacked by Jakub can be ignored.)
Ack to all.
I think even the first one should be pushed. 1) I don't think we are going to
use talloc pools anytime soon. 2) When and if we are going to use them it
will be a big change and in my opinion it will be more doable to convert
consistent code.
Consistent code does not require following change in the 1st patch
- tmp_ctx = talloc_new(mem_ctx);
+ tmp_ctx = talloc_new(NULL);
temporary context needn't be allocated on NULL context
IMHO; it can be allocated on mem_ctx without any issue.
LS