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

2013-03-11 Thread 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

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

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

2013-03-08 Thread 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
>From 5bffce519949c5b394a2d0c4a09bd0d5d9b258bf Mon Sep 17 00:00:00 2001
From: Tomas Babej 
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 ex

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

2013-03-08 Thread Tomas Babej 

On 03/07/2013 11:48 PM, Rob Crittenden wrote:

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

Tomas



So I assume in your reproduction steps the secondary range for both is 
0 hence the overlap?

Yes, that causes the issue.


rob


Tomas

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

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

2013-03-08 Thread Martin Kosek 

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

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

2013-03-07 Thread Rob Crittenden 

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

Tomas



So I assume in your reproduction steps the secondary range for both is 0 
hence the overlap?


rob

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

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

2013-03-05 Thread Tomas Babej 

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

Tomas
>From 1a18bc43b561a1bbcfa1f5da3c2f1d6482571d18 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
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 | 19 +--
 1 file changed, 13 insertions(+), 6 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..b7e0ea7af74e761dedd42d71d3b2ca7bc8aa3655 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,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;
 
 /* 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))
+r1->id_range_size, r2->id_range_size) && local_ranges)
 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))
+r1->id_range_size, r2->id_range_size) && local_ranges)
 return 5;
 }
 
-- 
1.7.11.7

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