<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1: Code-Review-1</p><p style="white-space: pre-wrap; word-wrap: break-word;">(1 comment)</p></blockquote><pre style="font-family: monospace,monospace; white-space: pre-wrap;">It's not obvious and complicated to understand looking at code.<br>The function sip_options_synchronize_endpoint uses the 'aor' parameter as a filter only in once case - when AOR is added.<br>I think better to have a function to filter endpoints by aor and function sip_options_synchronize_endpoint should do just what it is for, i.e. synchronize endpoint.<br>It's logging<br>ast_debug(3, "Synchronizing endpoint '%s' with AORs '%s'\n",<br>                ast_sorcery_object_get_id(endpoint), endpoint->aors);<br>even if does nothing</pre></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Were you envisioning filtering the endpoints container so it only contains endpoints referencing the AOR, and then a second iteration is done just for those endpoints? The consequence of that would be it would be less efficient. Or were you envisioning going through, doing the AOR check, and calling synchronize function if it passes?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">In real world how many endpoints have more then 1 AOR and how many endpoints have the same AOR?<br>May be it'll be a little inefficient, but will prevent errors due to implicit coding.<br></p><p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ddd; color: #000000;">-Code-Review</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14056">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/14056">change 14056</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/14056"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I3ee00023be2393113cd4e056599f23f3499ef164 </div>
<div style="display:none"> Gerrit-Change-Number: 14056 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Alexei Gradinari <alex2grad@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 31 Mar 2020 17:15:21 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>