Re: [SSSD] [PATCHES] fix minor memory leaks

2015-10-02 Thread Pavel Březina 

On 10/02/2015 10:42 AM, Pavel Reichl wrote:



On 09/30/2015 09:21 AM, Pavel Reichl wrote:



On 09/29/2015 05:49 PM, Jakub Hrozek wrote:

On Tue, Sep 29, 2015 at 02:20:39PM +0200, Pavel Reichl wrote:

In my opinion we can drop the change. This is not an imminent bug
it's rather code style dispute and I don't think that we have to
bother Sumit for that. Unless of course pbrezina feels different.


I feel like God :D



I don't mind too much, it just doesn't seem to be helpful change, that's
all, so I'll be a bit happier if we drop it.


Updated patch set attached. I removed the discussed 'talloc_new' line
from 1st patch.


Pavel would you re-ack the 1st patch (the rest of the patchset is
unchanged), please?


Ack to all.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-10-02 Thread Pavel Reichl 



On 09/30/2015 09:21 AM, Pavel Reichl wrote:



On 09/29/2015 05:49 PM, Jakub Hrozek wrote:

On Tue, Sep 29, 2015 at 02:20:39PM +0200, Pavel Reichl wrote:

In my opinion we can drop the change. This is not an imminent bug it's rather 
code style dispute and I don't think that we have to bother Sumit for that. 
Unless of course pbrezina feels different.


I don't mind too much, it just doesn't seem to be helpful change, that's
all, so I'll be a bit happier if we drop it.


Updated patch set attached. I removed the discussed 'talloc_new' line from 1st 
patch.


Pavel would you re-ack the 1st patch (the rest of the patchset is unchanged), 
please?



Bye.


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-10-02 Thread Jakub Hrozek 
On Fri, Oct 02, 2015 at 11:42:49AM +0200, Pavel Březina wrote:
> Ack to all.

* master:
* 8c9ecf0bd04be87a61d5f0e490ab8a7c48f481dd
* 3fa03d5816d6a401d8e894b77236d3cfd95dbd96
* a2d6d4db64a7c3b27dea22fe52245925d688bd2d
* 12440d2acbeb7ea6e5c0e4182d00377c8d01185b
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-30 Thread Pavel Reichl 



On 09/29/2015 05:49 PM, Jakub Hrozek wrote:

On Tue, Sep 29, 2015 at 02:20:39PM +0200, Pavel Reichl wrote:

In my opinion we can drop the change. This is not an imminent bug it's rather 
code style dispute and I don't think that we have to bother Sumit for that. 
Unless of course pbrezina feels different.


I don't mind too much, it just doesn't seem to be helpful change, that's
all, so I'll be a bit happier if we drop it.


Updated patch set attached. I removed the discussed 'talloc_new' line from 1st 
patch.

Bye.
>From 9cae5048afcfe16b053aa8322a275a9682d8bcdc Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:02:42 -0400
Subject: [PATCH 1/4] AD: fix minor memory leak

---
 src/providers/ad/ad_common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index 130cdeb613aae3843f7453a478815daaae6aab77..88f36f8ead64443e07b2393aa22ad4d9708d0a0e 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -745,7 +745,7 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx,
 ret = be_add_online_cb(bectx, bectx, ad_online_cb, service, NULL);
 if (ret != EOK) {
 DEBUG(SSSDBG_CRIT_FAILURE, "Could not set up AD online callback\n");
-return ret;
+goto done;
 }
 
 ret = be_fo_service_add_callback(mem_ctx, bectx, ad_service,
@@ -797,7 +797,8 @@ ad_resolve_callback(void *private_data, struct fo_server *server)
 sdata = fo_get_server_user_data(server);
 if (fo_is_srv_lookup(server) == false && sdata == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "No user data?\n");
-return;
+ret = EINVAL;
+goto done;
 }
 
 service = talloc_get_type(private_data, struct ad_service);
-- 
2.4.3

>From 5a20e0120278dba83381cc41f95024f2850ff745 Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:03:45 -0400
Subject: [PATCH 2/4] IPA: fix minor memory leak

---
 src/providers/ipa/ipa_hbac_rules.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_hbac_rules.c b/src/providers/ipa/ipa_hbac_rules.c
index ffef6dc4ce4229f2063d1b00308892bd3765f398..6ce1f76bb656352ec61332007a6fb62568374203 100644
--- a/src/providers/ipa/ipa_hbac_rules.c
+++ b/src/providers/ipa/ipa_hbac_rules.c
@@ -86,7 +86,7 @@ ipa_hbac_rule_info_send(TALLOC_CTX *mem_ctx,
 req = tevent_req_create(mem_ctx, , struct ipa_hbac_rule_state);
 if (req == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create failed.\n");
-return NULL;
+goto error;
 }
 
 state->ev = ev;
-- 
2.4.3

>From d33c71c28261f357965344e6c48a989cf168e060 Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:04:10 -0400
Subject: [PATCH 3/4] SDAP: fix minor memory leak

---
 src/providers/ldap/sdap_async_groups.c | 2 +-
 src/providers/ldap/sdap_idmap.c| 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 653187b3add172d37f234850ccab8ddf109e229c..609668339b7d7d37e578bd3cca8f8c7cac1e6671 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -592,7 +592,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
 if (ret != EOK) {
 DEBUG(SSSDBG_OP_FAILURE,
   "Error: Failed to mark group as non-posix!\n");
-return ret;
+goto done;
 }
 }
 
diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
index dd959b2c133b342f105f76c26c889d678ce40391..36d529836eb7e4becd27721df15cfbccf035ae3b 100644
--- a/src/providers/ldap/sdap_idmap.c
+++ b/src/providers/ldap/sdap_idmap.c
@@ -206,7 +206,8 @@ sdap_idmap_init(TALLOC_CTX *mem_ctx,
 if (err != IDMAP_SUCCESS) {
 /* This should never happen */
 DEBUG(SSSDBG_CRIT_FAILURE, "sss_idmap_ctx corrupted\n");
-return EIO;
+ret = EIO;
+goto done;
 }
 
 
-- 
2.4.3

>From e63211aaa15887c78deee5a76ed60684f9e6aaf7 Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:04:40 -0400
Subject: [PATCH 4/4] PROXY: fix minor memory leak

---
 src/providers/proxy/proxy_services.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/proxy/proxy_services.c b/src/providers/proxy/proxy_services.c
index 2aa44dbf7f061b99a551b199d0265393e5399a06..ee545cedc9f8dc36981eb3190ff16763e1da390f 100644
--- a/src/providers/proxy/proxy_services.c
+++ b/src/providers/proxy/proxy_services.c
@@ -130,7 +130,7 @@ get_serv_byname(struct proxy_id_ctx *ctx,
 if (status != NSS_STATUS_SUCCESS && status != NSS_STATUS_NOTFOUND) {
 DEBUG(SSSDBG_MINOR_FAILURE,
   "getservbyname_r failed for service [%s].\n", name);
-return ret;
+goto done;
 }
 
 if (status ==

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-29 Thread Pavel Reichl 



On 09/29/2015 02:01 PM, Lukas Slebodnik wrote:

On (29/09/15 13:32), Pavel Reichl wrote:



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 
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



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?

It would be good to to also push 1st patch
without change
-tmp_ctx = talloc_new(mem_ctx);
+tmp_ctx = talloc_new(NULL);



OK, I'll amend the patch.



There were two -1 for the first patch (jhrozek, lslebodn)
and one +1 from pbrezina.

If we consider you an author then there is another +1.

Do we need another (5th person) to decide which way is better?
or are two -1s enough to remove such change?


In my opinion we can drop the change. This is not an imminent bug it's rather 
code style dispute and I don't think that we have to bother Sumit for that. 
Unless of course pbrezina feels different.



LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-29 Thread Lukas Slebodnik 
On (29/09/15 13:32), Pavel Reichl wrote:
>
>
>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 
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
>>
>
>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?
It would be good to to also push 1st patch
without change
-tmp_ctx = talloc_new(mem_ctx);
+tmp_ctx = talloc_new(NULL);


There were two -1 for the first patch (jhrozek, lslebodn)
and one +1 from pbrezina.

If we consider you an author then there is another +1.

Do we need another (5th person) to decide which way is better?
or are two -1s enough to remove such change?

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-29 Thread Pavel Reichl 



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 
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

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-29 Thread Jakub Hrozek 
On Tue, Sep 29, 2015 at 02:20:39PM +0200, Pavel Reichl wrote:
> 
> 
> On 09/29/2015 02:01 PM, Lukas Slebodnik wrote:
> >On (29/09/15 13:32), Pavel Reichl wrote:
> >>
> >>
> >>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 
> >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
> >>>
> >>
> >>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?
> >It would be good to to also push 1st patch
> >without change
> >-tmp_ctx = talloc_new(mem_ctx);
> >+tmp_ctx = talloc_new(NULL);
> >
> 
> OK, I'll amend the patch.
> 
> >
> >There were two -1 for the first patch (jhrozek, lslebodn)
> >and one +1 from pbrezina.
> >
> >If we consider you an author then there is another +1.
> >
> >Do we need another (5th person) to decide which way is better?
> >or are two -1s enough to remove such change?
> 
> In my opinion we can drop the change. This is not an imminent bug it's rather 
> code style dispute and I don't think that we have to bother Sumit for that. 
> Unless of course pbrezina feels different.

I don't mind too much, it just doesn't seem to be helpful change, that's
all, so I'll be a bit happier if we drop it.

On the other hand, if we decide to use talloc pools, then we'll change
all or most NULL contexts, which would essentially revert this change.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-21 Thread Lukas Slebodnik 
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 
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

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-21 Thread Pavel Březina 

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 
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.


It can. It is not our coding style though.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-21 Thread Pavel Březina 

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 
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.


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-21 Thread Lukas Slebodnik 
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 
>>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

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-20 Thread Pavel Reichl 



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

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-04 Thread Pavel Reichl 



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 
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.
OK, I just find the usage of tmp_ctx somewhat awkward in this function. I think function would be better off without it completely - service would be allocated directly on mem_ctx. But I agree this is not a bug and possibly just a matter of personal 
preference, so we can ignore that.




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


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-04 Thread Jakub Hrozek 
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 
> 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

[SSSD] [PATCHES] fix minor memory leaks

2015-09-04 Thread Pavel Reichl 

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 
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);
 if (!tmp_ctx) return ENOMEM;
 
 service = talloc_zero(tmp_ctx, struct ad_service);
@@ -745,7 +745,7 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx,
 ret = be_add_online_cb(bectx, bectx, ad_online_cb, service, NULL);
 if (ret != EOK) {
 DEBUG(SSSDBG_CRIT_FAILURE, "Could not set up AD online callback\n");
-return ret;
+goto done;
 }
 
 ret = be_fo_service_add_callback(mem_ctx, bectx, ad_service,
@@ -797,7 +797,8 @@ ad_resolve_callback(void *private_data, struct fo_server *server)
 sdata = fo_get_server_user_data(server);
 if (fo_is_srv_lookup(server) == false && sdata == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "No user data?\n");
-return;
+ret = EINVAL;
+goto done;
 }
 
 service = talloc_get_type(private_data, struct ad_service);
-- 
2.4.3

>From 7533dde984c08c0580b924c0f480e6fd17c34395 Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:03:45 -0400
Subject: [PATCH 2/4] IPA: fix minor memory leak

---
 src/providers/ipa/ipa_hbac_rules.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_hbac_rules.c b/src/providers/ipa/ipa_hbac_rules.c
index ffef6dc4ce4229f2063d1b00308892bd3765f398..6ce1f76bb656352ec61332007a6fb62568374203 100644
--- a/src/providers/ipa/ipa_hbac_rules.c
+++ b/src/providers/ipa/ipa_hbac_rules.c
@@ -86,7 +86,7 @@ ipa_hbac_rule_info_send(TALLOC_CTX *mem_ctx,
 req = tevent_req_create(mem_ctx, , struct ipa_hbac_rule_state);
 if (req == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create failed.\n");
-return NULL;
+goto error;
 }
 
 state->ev = ev;
-- 
2.4.3

>From 26433a65fef11bc40bb7e5bed1e9acd9e2adbdcf Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:04:10 -0400
Subject: [PATCH 3/4] SDAP: fix minor memory leak

---
 src/providers/ldap/sdap_async_groups.c | 2 +-
 src/providers/ldap/sdap_idmap.c| 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 525c6fa09553d8c0232ce2317751184f83632d86..52487e3cbc6c67c41966cd6a3665c53d30cd4055 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -592,7 +592,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
 if (ret != EOK) {
 DEBUG(SSSDBG_OP_FAILURE,
   "Error: Failed to mark group as non-posix!\n");
-return ret;
+goto done;
 }
 }
 
diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
index dd959b2c133b342f105f76c26c889d678ce40391..36d529836eb7e4becd27721df15cfbccf035ae3b 100644
--- a/src/providers/ldap/sdap_idmap.c
+++ b/src/providers/ldap/sdap_idmap.c
@@ -206,7 +206,8 @@ sdap_idmap_init(TALLOC_CTX *mem_ctx,
 if (err != IDMAP_SUCCESS) {
 /* This should never happen */
 DEBUG(SSSDBG_CRIT_FAILURE, "sss_idmap_ctx corrupted\n");
-return EIO;
+ret = EIO;
+goto done;
 }
 
 
-- 
2.4.3

>From 4227817656ce966ecf82fd120f83b623cf31ea7a Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:04:40 -0400
Subject: [PATCH 4/4] PROXY: fix minor memory leak

---
 src/providers/proxy/proxy_services.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/proxy/proxy_services.c b/src/providers/proxy/proxy_services.c
index 2aa44dbf7f061b99a551b199d0265393e5399a06..ee545cedc9f8dc36981eb3190ff16763e1da390f 100644
--- a/src/providers/proxy/proxy_services.c
+++ b/src/providers/proxy/proxy_services.c
@@ -130,7 +130,7 @@ get_serv_byname(struct proxy_id_ctx *ctx,
 if (status != NSS_STATUS_SUCCESS && status != NSS_STATUS_NOTFOUND) {
 DEBUG(SSSDBG_MINOR_FAILURE,
   "getservbyname_r failed for service [%s].\n", name);
-return ret;
+goto done;
 }
 
 if (status == NSS_STATUS_NOTFOUND) {
@@ -183,7 +183,7 @@ get_serv_byport(struct proxy_id_ctx *ctx,