On 07/14/2014 11:43 AM, Tomas Babej wrote:

On 07/12/2014 08:22 PM, Lukas Slebodnik wrote:
On (01/04/14 10:52), Tomas Babej wrote:
On 04/01/2014 10:40 AM, Alexander Bokovoy wrote:
On Tue, 01 Apr 2014, Tomas Babej wrote:
 From 736b3f747188696fd4a46ca63d91a6cca942fd56 Mon Sep 17 00:00:00 2001
From: Tomas Babej <[email protected]>
Date: Wed, 5 Mar 2014 12:28:18 +0100
Subject: [PATCH] Extend ipa-range-check DS plugin to handle range types

The ipa-range-check plugin used to determine the range type depending
on the value of the attributes such as RID or secondary RID base. This
approached caused variety of issues since the portfolio of ID range
types expanded.

The patch makes sure the following rules are implemented:
    * No ID range pair can overlap on base ranges, with exception
      of two ipa-ad-trust-posix ranges belonging to the same forest
    * For any ID range pair of ranges belonging to the same domain:
        * Both ID ranges must be of the same type
        * For ranges of ipa-ad-trust type or ipa-local type:
            * Primary RID ranges can not overlap
        * For ranges of ipa-local type:
            * Primary and secondary RID ranges can not overlap
            * Secondary RID ranges cannot overlap

For the implementation part, the plugin was extended with a domain ID
to forest root domain ID mapping derivation capabilities.

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

-static int slapi_entry_to_range_info(struct slapi_entry *entry,
+struct domain_info {
+    char *domain_id;
+    char *forest_root_id;
+    struct domain_info *next;
+};
+
+static void free_domain_info(struct domain_info *info) {
+    if (info != NULL) {
+        slapi_ch_free_string(&(info->domain_id));
+        slapi_ch_free_string(&(info->forest_root_id));
+        free_domain_info(info->next);
+        free(info);
+    }
+}
Please, don't use recursion in the freeing part, there is really no
pressing need to do so. Just use while() like you do in
get_forest_root_id():

+/* Searches for the domain_info struct with the specified domain_id
+ * in the linked list. Returns the forest root domain's ID, or NULL for
+ * local ranges. */
+
+static char* get_forest_root_id(struct domain_info *head, char*
domain_id) {
+
+    /* For local ranges there is no forest root domain,
+     * so consider only ranges with domain_id set */
+    if (domain_id != NULL) {
+        while(head) {
+            if (strcasecmp(head->domain_id, domain_id) == 0) {
+                return head->forest_root_id;
+            }
+            head = head->next;
+        }
+     }
+
+    return NULL;
+}
+

Fixed, updated patch attached.

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

>From f98082d6cfd902417af94d7cd22d3cc85980a782 Mon Sep 17 00:00:00 2001
From: Tomas Babej <[email protected]>
Date: Wed, 5 Mar 2014 12:28:18 +0100
Subject: [PATCH] Extend ipa-range-check DS plugin to handle range types

The ipa-range-check plugin used to determine the range type depending
on the value of the attributes such as RID or secondary RID base. This
approached caused variety of issues since the portfolio of ID range
types expanded.

The patch makes sure the following rules are implemented:
    * No ID range pair can overlap on base ranges, with exception
      of two ipa-ad-trust-posix ranges belonging to the same forest
    * For any ID range pair of ranges belonging to the same domain:
        * Both ID ranges must be of the same type
        * For ranges of ipa-ad-trust type or ipa-local type:
            * Primary RID ranges can not overlap
        * For ranges of ipa-local type:
            * Primary and secondary RID ranges can not overlap
            * Secondary RID ranges cannot overlap

For the implementation part, the plugin was extended with a domain ID
to forest root domain ID mapping derivation capabilities.

https://fedorahosted.org/freeipa/ticket/4137
---
//snip

-        no_overlap = ranges_overlap(new_range, old_range);
+        ranges_valid = check_ranges(new_range, old_range);
         free_range_info(old_range);
         old_range = NULL;
-        if (no_overlap != 0) {
+        if (ranges_valid != 0) {
             ret = LDAP_CONSTRAINT_VIOLATION;

-            switch (no_overlap){
+            switch (ranges_valid){
             case 1:
                 errmsg = "New base range overlaps with existing base range.";
                 break;
@@ -417,6 +627,8 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int 
modtype)
             case 5:
                 errmsg = "New secondary rid range overlaps with existing primary 
rid range.";
                 break;
+            case 6:
+                errmsg = "New ID range has invalid type. All ranges in the same 
domain must be of the same type.";
             default:
There is a missing break. Ithink it is a problem, because you do not
want to fall through to the defaul and override errmsg.

Smple patch is attached.

LS

ACK, thanks Lukas!


Pushed to master, ipa-4-1, ipa-4-0: d1d253637535017fdf82aa833df742bb418bbf65


--
Petr³

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to