[SSSD] [PATCHES] fix minor memory leaks

Pavel Březina pbrezina at redhat.com
Mon Sep 21 08:59:17 UTC 2015


On 09/21/2015 10:58 AM, Lukas Slebodnik wrote:
> 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 at 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 at 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.



More information about the sssd-devel mailing list