On 09/21/2015 11:16 AM, Lukas Slebodnik wrote:
On (21/09/15 10:59), Pavel Březina wrote:
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 <prei...@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;
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.
But it needn't :-)
So this change is not necessary.
I checked the source code and in 10% of cases we do not use
NULL context for temporary context. Moreover if we want to use talloc pools
in future then it would be better to reduce usage of NULL context.
It's fine to use NULL context for allocation in function which can be unit
tested (leak_check_setup, leak_check_teardown); but it is not this case
in the 1st patch.
LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
As the discussion about the first patch stalled I propose to omit it and push
the rest of the patch set to master.
Should I send updated patch set or committer can skip the first patch himself?
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
