[Freeipa-devel] [PATCH 0038] Perform secondary rid range overlap check for local ranges

[Martin Kosek]

On 03/08/2013 04:41 PM, Tomas Babej wrote:
> 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

ACK. Pushed to master, ipa-3-1.

Martin