Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-08 Thread Martin Kosek
On 04/03/2014 04:31 PM, Alexander Bokovoy wrote:
 On Wed, 02 Apr 2014, Martin Kosek wrote:
 On 04/01/2014 12:03 PM, Jan Pazdziora wrote:
 On Tue, Apr 01, 2014 at 10:05:39AM +0200, Tomas Babej wrote:

 Yes, that was the intention. Mistake on my part, I'll send updated 
 patches.


 Updated patch attached.

 Ack based on reading the code and documentation for
 slapi_ch_free_string.


 Ok, thanks. Though I would like this patch to be also functionally tested 
 that
 it does not break anything, ideally together with your other ipa-range 
 patches.
 ACK to 0162, 0161, 0158 (should be applied in this order).
 
 # ipa idrange-find
 
 2 ranges matched
 
 Range name: AD.TEST_id_range
 First Posix ID of the range: 111500
 Number of IDs in the range: 20
 First RID of the corresponding RID range: 0
 Domain SID of the trusted domain:  S-1-5-21-2275361654-3393353068-3720134936
 Range type: Active Directory domain range
 
 Range name: T.VDA.LI_id_range
 First Posix ID of the range: 91740
 Number of IDs in the range: 20
 First RID of the corresponding RID range: 1000
 First RID of the secondary RID range: 1
 Range type: local domain range
 
 Number of entries returned 2
 
 # ipa idrange-add AD.TEST_1_id_range
 First Posix ID of the range: 111900
 Number of IDs in the range: 20
 First RID of the corresponding RID range: 0
 First RID of the secondary RID range: 100
 ipa: ERROR: Constraint violation: New primary rid range overlaps with existing
 primary rid range.
 
 the message comes from the ipa-range-check plugin.
 
 

Pushed all 3 to master.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-03 Thread Alexander Bokovoy

On Wed, 02 Apr 2014, Martin Kosek wrote:

On 04/01/2014 12:03 PM, Jan Pazdziora wrote:

On Tue, Apr 01, 2014 at 10:05:39AM +0200, Tomas Babej wrote:



Yes, that was the intention. Mistake on my part, I'll send updated patches.



Updated patch attached.


Ack based on reading the code and documentation for
slapi_ch_free_string.



Ok, thanks. Though I would like this patch to be also functionally tested that
it does not break anything, ideally together with your other ipa-range patches.

ACK to 0162, 0161, 0158 (should be applied in this order).

# ipa idrange-find

2 ranges matched

Range name: AD.TEST_id_range
First Posix ID of the range: 111500
Number of IDs in the range: 20
First RID of the corresponding RID range: 0
Domain SID of the trusted domain:  S-1-5-21-2275361654-3393353068-3720134936
Range type: Active Directory domain range

Range name: T.VDA.LI_id_range
First Posix ID of the range: 91740
Number of IDs in the range: 20
First RID of the corresponding RID range: 1000
First RID of the secondary RID range: 1
Range type: local domain range

Number of entries returned 2

# ipa idrange-add AD.TEST_1_id_range
First Posix ID of the range: 111900
Number of IDs in the range: 20
First RID of the corresponding RID range: 0
First RID of the secondary RID range: 100
ipa: ERROR: Constraint violation: New primary rid range overlaps with existing 
primary rid range.

the message comes from the ipa-range-check plugin.


--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-02 Thread Martin Kosek
On 04/01/2014 12:03 PM, Jan Pazdziora wrote:
 On Tue, Apr 01, 2014 at 10:05:39AM +0200, Tomas Babej wrote:

 Yes, that was the intention. Mistake on my part, I'll send updated patches.


 Updated patch attached.
 
 Ack based on reading the code and documentation for
 slapi_ch_free_string.
 

Ok, thanks. Though I would like this patch to be also functionally tested that
it does not break anything, ideally together with your other ipa-range patches.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-02 Thread Alexander Bokovoy

On Wed, 02 Apr 2014, Martin Kosek wrote:

On 04/01/2014 12:03 PM, Jan Pazdziora wrote:

On Tue, Apr 01, 2014 at 10:05:39AM +0200, Tomas Babej wrote:



Yes, that was the intention. Mistake on my part, I'll send updated patches.



Updated patch attached.


Ack based on reading the code and documentation for
slapi_ch_free_string.



Ok, thanks. Though I would like this patch to be also functionally tested that
it does not break anything, ideally together with your other ipa-range patches.

It is on my test list, don't worry.

--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-01 Thread Jan Pazdziora
On Thu, Mar 27, 2014 at 01:14:52PM +0100, Tomas Babej wrote:
 Hi,
 
 When cleaning the range_info struct, simple free of the struct
 is not enough, we have to free contents of char pointers in the
 struct as well.
 
 https://fedorahosted.org/freeipa/ticket/4276

[...]


  1 file changed, 13 insertions(+), 4 deletions(-)
 
 diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c 
 b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 index 
 0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc
  100644
 --- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 +++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 @@ -96,6 +96,15 @@ struct domain_info {
  struct domain_info *next;
  };
  
 +static void free_range_info(struct range_info *range) {
 +if (range != NULL) {
 +slapi_ch_free_string((range-name));
 +slapi_ch_free_string((range-domain_id));
 +slapi_ch_free_string((range-forest_root_id));
 +slapi_ch_free_string((range-id_range_type));
 +free(range);
 +}
 +}

In master, the range_info is

struct range_info {
char *name;
char *domain_id;
uint32_t base_id;
uint32_t id_range_size;
uint32_t base_rid;
uint32_t secondary_base_rid;
};

-- no forest_root_id and no id_range_type.

So NACK for applying to master. Perhaps there is some dependency
patch?

-- 
Jan Pazdziora
Principal Software Engineer, Identity Management Engineering, Red Hat

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-01 Thread Alexander Bokovoy

On Tue, 01 Apr 2014, Jan Pazdziora wrote:

On Thu, Mar 27, 2014 at 01:14:52PM +0100, Tomas Babej wrote:

Hi,

When cleaning the range_info struct, simple free of the struct
is not enough, we have to free contents of char pointers in the
struct as well.

https://fedorahosted.org/freeipa/ticket/4276


[...]



 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c 
b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
index 
0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc
 100644
--- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
+++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
@@ -96,6 +96,15 @@ struct domain_info {
 struct domain_info *next;
 };

+static void free_range_info(struct range_info *range) {
+if (range != NULL) {
+slapi_ch_free_string((range-name));
+slapi_ch_free_string((range-domain_id));
+slapi_ch_free_string((range-forest_root_id));
+slapi_ch_free_string((range-id_range_type));
+free(range);
+}
+}


In master, the range_info is

struct range_info {
char *name;
char *domain_id;
uint32_t base_id;
uint32_t id_range_size;
uint32_t base_rid;
uint32_t secondary_base_rid;
};

-- no forest_root_id and no id_range_type.

So NACK for applying to master. Perhaps there is some dependency
patch?

I think Tomas took his own range check patch into account.

I would also prefer splitting their dependencies. First make this one to
work on master, then add forest_root_id and id_range_type freeing as
part of the range check patches.

--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-01 Thread Tomas Babej

On 04/01/2014 09:11 AM, Alexander Bokovoy wrote:
 On Tue, 01 Apr 2014, Jan Pazdziora wrote:
 On Thu, Mar 27, 2014 at 01:14:52PM +0100, Tomas Babej wrote:
 Hi,

 When cleaning the range_info struct, simple free of the struct
 is not enough, we have to free contents of char pointers in the
 struct as well.

 https://fedorahosted.org/freeipa/ticket/4276

 [...]


  1 file changed, 13 insertions(+), 4 deletions(-)

 diff --git
 a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 index
 0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc
 100644
 --- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 +++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 @@ -96,6 +96,15 @@ struct domain_info {
  struct domain_info *next;
  };

 +static void free_range_info(struct range_info *range) {
 +if (range != NULL) {
 +slapi_ch_free_string((range-name));
 +slapi_ch_free_string((range-domain_id));
 +slapi_ch_free_string((range-forest_root_id));
 +slapi_ch_free_string((range-id_range_type));
 +free(range);
 +}
 +}

 In master, the range_info is

 struct range_info {
 char *name;
 char *domain_id;
 uint32_t base_id;
 uint32_t id_range_size;
 uint32_t base_rid;
 uint32_t secondary_base_rid;
 };

 -- no forest_root_id and no id_range_type.

 So NACK for applying to master. Perhaps there is some dependency
 patch?
 I think Tomas took his own range check patch into account.

 I would also prefer splitting their dependencies. First make this one to
 work on master, then add forest_root_id and id_range_type freeing as
 part of the range check patches.


Yes, that was the intention. Mistake on my part, I'll send updated patches.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-01 Thread Tomas Babej

On 04/01/2014 09:17 AM, Tomas Babej wrote:
 On 04/01/2014 09:11 AM, Alexander Bokovoy wrote:
 On Tue, 01 Apr 2014, Jan Pazdziora wrote:
 On Thu, Mar 27, 2014 at 01:14:52PM +0100, Tomas Babej wrote:
 Hi,

 When cleaning the range_info struct, simple free of the struct
 is not enough, we have to free contents of char pointers in the
 struct as well.

 https://fedorahosted.org/freeipa/ticket/4276
 [...]


  1 file changed, 13 insertions(+), 4 deletions(-)

 diff --git
 a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 index
 0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc
 100644
 --- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 +++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
 @@ -96,6 +96,15 @@ struct domain_info {
  struct domain_info *next;
  };

 +static void free_range_info(struct range_info *range) {
 +if (range != NULL) {
 +slapi_ch_free_string((range-name));
 +slapi_ch_free_string((range-domain_id));
 +slapi_ch_free_string((range-forest_root_id));
 +slapi_ch_free_string((range-id_range_type));
 +free(range);
 +}
 +}
 In master, the range_info is

 struct range_info {
 char *name;
 char *domain_id;
 uint32_t base_id;
 uint32_t id_range_size;
 uint32_t base_rid;
 uint32_t secondary_base_rid;
 };

 -- no forest_root_id and no id_range_type.

 So NACK for applying to master. Perhaps there is some dependency
 patch?
 I think Tomas took his own range check patch into account.

 I would also prefer splitting their dependencies. First make this one to
 work on master, then add forest_root_id and id_range_type freeing as
 part of the range check patches.

 Yes, that was the intention. Mistake on my part, I'll send updated patches.


Updated patch attached.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

From d6d0d8a05fe138ec93e6dc0c6015e59c040aca78 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 1 Apr 2014 09:20:09 +0200
Subject: [PATCH] ipa-range-check: Fix memory leaks when freeing range object

When cleaning the range_info struct, simple free of the struct
is not enough, we have to free contents of char pointers in the
struct as well.

https://fedorahosted.org/freeipa/ticket/4276
---
 .../ipa-slapi-plugins/ipa-range-check/ipa_range_check.c  | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
index 391e2259b6eced31fed399c927cfe44c1d3e237e..4f0640eafdaed38cbf72beccc146009a599fa108 100644
--- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
+++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
@@ -78,6 +78,14 @@ struct range_info {
 uint32_t secondary_base_rid;
 };
 
+static void free_range_info(struct range_info *range) {
+if (range != NULL) {
+slapi_ch_free_string((range-name));
+slapi_ch_free_string((range-domain_id));
+free(range);
+}
+}
+
 static int slapi_entry_to_range_info(struct slapi_entry *entry,
  struct range_info **_range)
 {
@@ -131,7 +139,7 @@ static int slapi_entry_to_range_info(struct slapi_entry *entry,
 
 done:
 if (ret != 0) {
-free(range);
+free_range_info(range);
 }
 
 return ret;
@@ -388,7 +396,7 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
 }
 
 no_overlap = ranges_overlap(new_range, old_range);
-free(old_range);
+free_range_info(old_range);
 old_range = NULL;
 if (no_overlap != 0) {
 ret = LDAP_CONSTRAINT_VIOLATION;
@@ -426,8 +434,8 @@ done:
 slapi_free_search_results_internal(search_pb);
 slapi_pblock_destroy(search_pb);
 slapi_sdn_free(dn);
-free(old_range);
-free(new_range);
+free_range_info(old_range);
+free_range_info(new_range);
 if (free_entry) {
 slapi_entry_free(entry);
 }
-- 
1.8.5.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-04-01 Thread Jan Pazdziora
On Tue, Apr 01, 2014 at 10:05:39AM +0200, Tomas Babej wrote:
 
  Yes, that was the intention. Mistake on my part, I'll send updated patches.
 
 
 Updated patch attached.

Ack based on reading the code and documentation for
slapi_ch_free_string.

-- 
Jan Pazdziora
Principal Software Engineer, Identity Management Engineering, Red Hat

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object

2014-03-27 Thread Tomas Babej
Hi,

When cleaning the range_info struct, simple free of the struct
is not enough, we have to free contents of char pointers in the
struct as well.

https://fedorahosted.org/freeipa/ticket/4276

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 


From 4752dd7cf5470e4da709ce0f31b9f8d408b79737 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 27 Mar 2014 13:03:33 +0100
Subject: [PATCH] ipa-range-check: Fix memory leaks when freeing range object

When cleaning the range_info struct, simple free of the struct
is not enough, we have to free contents of char pointers in the
struct as well.

https://fedorahosted.org/freeipa/ticket/4276
---
 .../ipa-slapi-plugins/ipa-range-check/ipa_range_check.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
index 0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc 100644
--- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
+++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
@@ -96,6 +96,15 @@ struct domain_info {
 struct domain_info *next;
 };
 
+static void free_range_info(struct range_info *range) {
+if (range != NULL) {
+slapi_ch_free_string((range-name));
+slapi_ch_free_string((range-domain_id));
+slapi_ch_free_string((range-forest_root_id));
+slapi_ch_free_string((range-id_range_type));
+free(range);
+}
+}
 
 static void free_domain_info(struct domain_info *info) {
 if (info != NULL) {
@@ -323,7 +332,7 @@ static int slapi_entry_to_range_info(struct domain_info *domain_info_head,
 
 done:
 if (ret != 0) {
-free(range);
+free_range_info(range);
 }
 
 return ret;
@@ -598,7 +607,7 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
 }
 
 ranges_valid = check_ranges(new_range, old_range);
-free(old_range);
+free_range_info(old_range);
 old_range = NULL;
 if (ranges_valid != 0) {
 ret = LDAP_CONSTRAINT_VIOLATION;
@@ -638,8 +647,8 @@ done:
 slapi_free_search_results_internal(search_pb);
 slapi_pblock_destroy(search_pb);
 slapi_sdn_free(dn);
-free(old_range);
-free(new_range);
+free_range_info(old_range);
+free_range_info(new_range);
 if (free_entry) {
 slapi_entry_free(entry);
 }
-- 
1.8.5.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel