<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Thanks,<br>
attached a new version with comments and trying to use more
meaningful function names<br>
<div class="moz-cite-prefix">On 06/11/2015 10:49 AM, thierry bordaz
wrote:<br>
</div>
<blockquote cite="mid:55794BB5.7040209@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 06/11/2015 10:40 AM, Ludwig
Krispenz wrote:<br>
</div>
<blockquote cite="mid:55794976.1000406@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<br>
<div class="moz-cite-prefix">On 06/11/2015 10:27 AM, thierry
bordaz wrote:<br>
</div>
<blockquote cite="mid:5579466D.9080305@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 06/11/2015 08:12 AM, Ludwig
Krispenz wrote:<br>
</div>
<blockquote cite="mid:557926CA.5090002@redhat.com" type="cite">Attached
are two patches: <br>
- reject direct modification of segment endpoints and
connectivity <br>
- better manage the rdn of a replication agreements
represented by a segment <br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
<font face="Times New Roman, Times, serif">Hello Ludwig,<br>
<br>
The patches looks good. Two questions:<br>
<br>
</font>
<ul>
<li><font face="Times New Roman, Times, serif">Patch 0012<br>
</font><tt> if (strcasecmp(agmt_rdn_str,
topo_agmt->rdn)) {</tt><tt><br>
</tt><tt>
slapi_ch_free_string(&topo_agmt->rdn);</tt><tt><br>
</tt><tt> topo_agmt->rdn =
slapi_ch_strdup(agmt_rdn_str);</tt><tt><br>
</tt><tt> }</tt><font face="Times New Roman,
Times, serif"><br>
What is the benefit of replacing a string by the same
one ?</font></li>
</ul>
</blockquote>
<font face="Times New Roman, Times, serif">if strcasecmp is not
0, they are not the same</font><br>
</blockquote>
Shame on me !<br>
<br>
<blockquote cite="mid:55794976.1000406@redhat.com" type="cite">
<blockquote cite="mid:5579466D.9080305@redhat.com" type="cite">
<ul>
<li><font face="Times New Roman, Times, serif">Patch 0013<br>
In ipa-topo-pre-mod.<br>
</font><tt> if (ipa_topo_is_entry_managed(pb)){</tt><tt><br>
</tt><tt> if(ipa_topo_is_agmt_attr_restricted(pb))
{</tt><tt><br>
</tt><tt> errtxt = slapi_ch_smprintf("Entry and
attributes are managed by topology plugin."</tt><tt><br>
</tt><tt> "No direct
modifications allowed.\n");</tt><tt><br>
</tt><tt> }</tt><tt><br>
</tt><tt> } else if
(ipa_topo_check_connect_restrict(pb)) {</tt><tt><br>
</tt><tt> errtxt = slapi_ch_smprintf("Modification
of connectivity and segment nodes "</tt><tt><br>
</tt><tt> " is not
supported.\n");</tt><tt><br>
</tt><tt> }</tt><font face="Times New Roman, Times,
serif"><br>
If we have an external modify of replication agreement
(managed by topology plugin), then it will not call '</font><font
face="Times New Roman, Times, serif"><tt>ipa_topo_check_connect_restrict</tt>'.<br>
And the modify will not be reject if for example it
updates 'ipaReplTopoSegmentDirection'.<br>
</font></li>
</ul>
</blockquote>
<font face="Times New Roman, Times, serif">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</font><br>
</blockquote>
<br>
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.<br>
<br>
Other than that the fix is good for me. ACK<br>
<br>
<br>
<blockquote cite="mid:55794976.1000406@redhat.com" type="cite">
<blockquote cite="mid:5579466D.9080305@redhat.com" type="cite">
<ul>
<li><font face="Times New Roman, Times, serif"> But I
thought that this patch also wants to prevent external
update of some connectivity attribute of the managed
entries.<br>
<br>
</font></li>
</ul>
<p><font face="Times New Roman, Times, serif">Thanks<br>
thierry<br>
</font></p>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>