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

[Tomas Babej]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0038-2-Perform-secondary-rid-range-overlap-check-for-local-.patch
Type: text/x-patch
Size: 3823 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130308/841cc543/attachment.bin>