On 03/08/2013 12:10 PM, Martin Kosek wrote:
On 03/05/2013 12:59 PM, Tomas Babej wrote:
Hi,

Any of the following checks:
   - overlap between primary RID range and secondary RID range
   - overlap between secondary RID range and secondary RID range

is performed now only if both of the ranges involved are local
domain ranges.

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


I think the patch is functionally OK (I tested it), I would just change the flow of the following:

@@ -194,19 +198,22 @@ static int ranges_overlap(struct range_info *r1, struct range_info *r2)
             r1->id_range_size, r2->id_range_size))
             return 2;

- /* check if secondary rid range overlaps with existing secondary rid range */
+        /**
+ * The following 3 checks are relevant only if both ranges are local. + * Check if secondary rid range overlaps with existing secondary rid
+         * range. **/
if (intervals_overlap(r1->secondary_base_rid, r2->secondary_base_rid,
-            r1->id_range_size, r2->id_range_size))
+            r1->id_range_size, r2->id_range_size) && local_ranges)
             return 3;
...


TO something like

...
        /**
* The following checks are relevant only if both ranges are local. * Check if secondary rid range overlaps with existing secondary rid
         * range. **/
        if (local_ranges) {
            ... do the checks
        }
...

Doing it your way, intervals_overlap() function is called 3 times when not needed + it is not so obvious that these checks are only done with "local_ranges" only.

Martin
Refactored.

Tomas
>From 5bffce519949c5b394a2d0c4a09bd0d5d9b258bf Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Tue, 5 Mar 2013 09:17:20 +0100
Subject: [PATCH] Perform secondary rid range overlap check for local ranges
 only

Any of the following checks:
  - overlap between primary RID range and secondary RID range
  - overlap between secondary RID range and secondary RID range

is performed now only if both of the ranges involved are local
domain ranges.

https://fedorahosted.org/freeipa/ticket/3391
---
 .../ipa-range-check/ipa_range_check.c              | 37 ++++++++++++++--------
 1 file changed, 23 insertions(+), 14 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 3a607636dc3ad9efc80ac7a2cef27eab524ad251..391e2259b6eced31fed399c927cfe44c1d3e237e 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
@@ -178,6 +178,11 @@ static int ranges_overlap(struct range_info *r1, struct range_info *r2)
     bool rid_ranges_set = (r1->base_rid != 0 || r1->secondary_base_rid != 0) &&
                           (r2->base_rid != 0 || r2->secondary_base_rid != 0);
 
+    /**
+     * ipaNTTrustedDomainSID is not set for local ranges, use it to
+     * determine the type of the range **/
+    bool local_ranges = r1->domain_id == NULL && r2->domain_id == NULL;
+
     bool ranges_from_same_domain =
          (r1->domain_id == NULL && r2->domain_id == NULL) ||
          (r1->domain_id != NULL && r2->domain_id != NULL &&
@@ -185,8 +190,7 @@ static int ranges_overlap(struct range_info *r1, struct range_info *r2)
 
     /**
      * in case rid range is not set or ranges belong to different domains
-     * we can skip rid range tests as they are irrelevant
-     */
+     * we can skip rid range tests as they are irrelevant **/
     if (rid_ranges_set && ranges_from_same_domain){
 
         /* check if rid range overlaps with existing rid range */
@@ -194,20 +198,25 @@ static int ranges_overlap(struct range_info *r1, struct range_info *r2)
             r1->id_range_size, r2->id_range_size))
             return 2;
 
-        /* check if secondary rid range overlaps with existing secondary rid range */
-        if (intervals_overlap(r1->secondary_base_rid, r2->secondary_base_rid,
-            r1->id_range_size, r2->id_range_size))
-            return 3;
+        /**
+         * The following 3 checks are relevant only if both ranges are local.
+         * Check if secondary rid range overlaps with existing secondary rid
+         * range. **/
+        if (local_ranges){
+            if (intervals_overlap(r1->secondary_base_rid,
+                r2->secondary_base_rid, r1->id_range_size, r2->id_range_size))
+                return 3;
 
-        /* check if rid range overlaps with existing secondary rid range */
-        if (intervals_overlap(r1->base_rid, r2->secondary_base_rid,
-            r1->id_range_size, r2->id_range_size))
-            return 4;
+            /* check if rid range overlaps with existing secondary rid range */
+            if (intervals_overlap(r1->base_rid, r2->secondary_base_rid,
+                r1->id_range_size, r2->id_range_size))
+                return 4;
 
-        /* check if secondary rid range overlaps with existing rid range */
-        if (intervals_overlap(r1->secondary_base_rid, r2->base_rid,
-            r1->id_range_size, r2->id_range_size))
-            return 5;
+            /* check if secondary rid range overlaps with existing rid range */
+            if (intervals_overlap(r1->secondary_base_rid, r2->base_rid,
+                r1->id_range_size, r2->id_range_size))
+                return 5;
+            }
     }
 
     return 0;
-- 
1.7.11.7

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

Reply via email to