On (01/04/14 10:52), Tomas Babej wrote:
>
>On 04/01/2014 10:40 AM, Alexander Bokovoy wrote:
>> On Tue, 01 Apr 2014, Tomas Babej wrote:
>>> From 736b3f747188696fd4a46ca63d91a6cca942fd56 Mon Sep 17 00:00:00 2001
>>> From: Tomas Babej <tba...@redhat.com>
>>> Date: Wed, 5 Mar 2014 12:28:18 +0100
>>> Subject: [PATCH] Extend ipa-range-check DS plugin to handle range types
>>>
>>> The ipa-range-check plugin used to determine the range type depending
>>> on the value of the attributes such as RID or secondary RID base. This
>>> approached caused variety of issues since the portfolio of ID range
>>> types expanded.
>>>
>>> The patch makes sure the following rules are implemented:
>>> * No ID range pair can overlap on base ranges, with exception
>>> of two ipa-ad-trust-posix ranges belonging to the same forest
>>> * For any ID range pair of ranges belonging to the same domain:
>>> * Both ID ranges must be of the same type
>>> * For ranges of ipa-ad-trust type or ipa-local type:
>>> * Primary RID ranges can not overlap
>>> * For ranges of ipa-local type:
>>> * Primary and secondary RID ranges can not overlap
>>> * Secondary RID ranges cannot overlap
>>>
>>> For the implementation part, the plugin was extended with a domain ID
>>> to forest root domain ID mapping derivation capabilities.
>>>
>>> https://fedorahosted.org/freeipa/ticket/4137
>>>
>>> -static int slapi_entry_to_range_info(struct slapi_entry *entry,
>>> +struct domain_info {
>>> + char *domain_id;
>>> + char *forest_root_id;
>>> + struct domain_info *next;
>>> +};
>>> +
>>> +static void free_domain_info(struct domain_info *info) {
>>> + if (info != NULL) {
>>> + slapi_ch_free_string(&(info->domain_id));
>>> + slapi_ch_free_string(&(info->forest_root_id));
>>> + free_domain_info(info->next);
>>> + free(info);
>>> + }
>>> +}
>> Please, don't use recursion in the freeing part, there is really no
>> pressing need to do so. Just use while() like you do in
>> get_forest_root_id():
>>
>>> +/* Searches for the domain_info struct with the specified domain_id
>>> + * in the linked list. Returns the forest root domain's ID, or NULL for
>>> + * local ranges. */
>>> +
>>> +static char* get_forest_root_id(struct domain_info *head, char*
>>> domain_id) {
>>> +
>>> + /* For local ranges there is no forest root domain,
>>> + * so consider only ranges with domain_id set */
>>> + if (domain_id != NULL) {
>>> + while(head) {
>>> + if (strcasecmp(head->domain_id, domain_id) == 0) {
>>> + return head->forest_root_id;
>>> + }
>>> + head = head->next;
>>> + }
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>
>>
>
>Fixed, updated patch attached.
>
>--
>Tomas Babej
>Associate Software Engineer | Red Hat | Identity Management
>RHCE | Brno Site | IRC: tbabej | freeipa.org
>
>>From f98082d6cfd902417af94d7cd22d3cc85980a782 Mon Sep 17 00:00:00 2001
>From: Tomas Babej <tba...@redhat.com>
>Date: Wed, 5 Mar 2014 12:28:18 +0100
>Subject: [PATCH] Extend ipa-range-check DS plugin to handle range types
>
>The ipa-range-check plugin used to determine the range type depending
>on the value of the attributes such as RID or secondary RID base. This
>approached caused variety of issues since the portfolio of ID range
>types expanded.
>
>The patch makes sure the following rules are implemented:
> * No ID range pair can overlap on base ranges, with exception
> of two ipa-ad-trust-posix ranges belonging to the same forest
> * For any ID range pair of ranges belonging to the same domain:
> * Both ID ranges must be of the same type
> * For ranges of ipa-ad-trust type or ipa-local type:
> * Primary RID ranges can not overlap
> * For ranges of ipa-local type:
> * Primary and secondary RID ranges can not overlap
> * Secondary RID ranges cannot overlap
>
>For the implementation part, the plugin was extended with a domain ID
>to forest root domain ID mapping derivation capabilities.
>
>https://fedorahosted.org/freeipa/ticket/4137
>---
//snip
>
>- no_overlap = ranges_overlap(new_range, old_range);
>+ ranges_valid = check_ranges(new_range, old_range);
> free_range_info(old_range);
> old_range = NULL;
>- if (no_overlap != 0) {
>+ if (ranges_valid != 0) {
> ret = LDAP_CONSTRAINT_VIOLATION;
>
>- switch (no_overlap){
>+ switch (ranges_valid){
> case 1:
> errmsg = "New base range overlaps with existing base range.";
> break;
>@@ -417,6 +627,8 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int
>modtype)
> case 5:
> errmsg = "New secondary rid range overlaps with existing
> primary rid range.";
> break;
>+ case 6:
>+ errmsg = "New ID range has invalid type. All ranges in the
>same domain must be of the same type.";
> default:
There is a missing break. Ithink it is a problem, because you do not
want to fall through to the defaul and override errmsg.
Smple patch is attached.
LS
>From 2f9060c011e7dd719887d7935a809887e26ac1e5 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Sat, 12 Jul 2014 18:28:38 +0200
Subject: [PATCH 2/4] Add missing break
Wrong error message would be used for in case of
RANGE_CHECK_DIFFERENT_TYPE_IN_DOMAIN. Missing break will cause fall through to
the default section.
---
daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c | 1 +
1 file changed, 1 insertion(+)
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
d659dc7d8b510cb5719dc9ff1db2ef55e2d237c4..1fd543c185cda0a7b871875f1d4ba969a7bf910c
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
@@ -660,6 +660,7 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int
modtype)
break;
case RANGE_CHECK_DIFFERENT_TYPE_IN_DOMAIN:
errmsg = "New ID range has invalid type. All ranges in the
same domain must be of the same type.";
+ break;
default:
errmsg = "New range overlaps with existing one.";
break;
--
1.9.3
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
