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.
It can. It is not our coding style though.