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

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

Reply via email to