[Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes

[Ludwig Krispenz]

Thanks,
attached a new version with comments and trying to use more meaningful 
function names
On 06/11/2015 10:49 AM, thierry bordaz wrote:
> On 06/11/2015 10:40 AM, Ludwig Krispenz wrote:
>>
>> On 06/11/2015 10:27 AM, thierry bordaz wrote:
>>> On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:
>>>> Attached are two patches:
>>>> - reject direct modification of segment endpoints and connectivity
>>>> - better manage the rdn of a replication agreements represented by 
>>>> a segment
>>>>
>>>>
>>> Hello Ludwig,
>>>
>>> The patches looks good. Two questions:
>>>
>>>   * Patch 0012
>>>                if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
>>>     slapi_ch_free_string(&topo_agmt->rdn);
>>>                     topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
>>>                 }
>>>     What is the benefit of replacing a string by the same one ?
>>>
>> if strcasecmp is not 0, they are not the same
> Shame on me !
>
>>>   * Patch 0013
>>>     In ipa-topo-pre-mod.
>>>         if (ipa_topo_is_entry_managed(pb)){
>>>             if(ipa_topo_is_agmt_attr_restricted(pb)) {
>>>                 errtxt = slapi_ch_smprintf("Entry and attributes are
>>>     managed by topology plugin."
>>>                                            "No direct modifications
>>>     allowed.\n");
>>>             }
>>>         } else if (ipa_topo_check_connect_restrict(pb)) {
>>>             errtxt = slapi_ch_smprintf("Modification of connectivity
>>>     and segment nodes "
>>>                                        " is not supported.\n");
>>>         }
>>>     If we have an external modify of replication agreement (managed
>>>     by topology plugin), then it will not call
>>>     'ipa_topo_check_connect_restrict'.
>>>     And the modify will not be reject if for example it updates
>>>     'ipaReplTopoSegmentDirection'.
>>>
>> this is not in an replication agreement. you cannot modify two 
>> entries in the same mod. The first if catches mos to a repl 
>> agreement, the second to a segment entry
>
> Ok. Thanks for the explanations. To help reading you may add a comment 
> saying the the first test is related to RA and the second to segments.
>
> Other than that the fix is good for me. ACK
>
>
>>>   * But I thought that this patch also wants to prevent external
>>>     update of some connectivity attribute of the managed entries.
>>>
>>> Thanks
>>> thierry
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150611/eeba92ac/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-lkrispen-0013-v2-reject-modifications-of-endpoints-and-connectivit.patch
Type: text/x-patch
Size: 5269 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150611/eeba92ac/attachment.bin>