On 07/12/2014 08:22 PM, Lukas Slebodnik wrote: > 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 <[email protected]> >>>> 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 <[email protected]> >> 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
ACK, thanks Lukas! -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
