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; >>>_______________________________________________ >>>sssd-devel mailing list >>>sssd-devel@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 _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel