Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object
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
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
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
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
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
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
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
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
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
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