[Asterisk-code-review] res_pjsip: Don't set endpoint to unavailable in all cases. (asterisk[16])

[Joshua Colp]

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14056 )

Change subject: res_pjsip: Don't set endpoint to unavailable in all cases.
......................................................................


Patch Set 1:

> Patch Set 1: -Code-Review
> 
> > Patch Set 1:
> > 
> > > Patch Set 1:
> > > 
> > > > Patch Set 1: Code-Review-1
> > > > 
> > > > (1 comment)
> > > 
> > > It's not obvious and complicated to understand looking at code.
> > > The function sip_options_synchronize_endpoint uses the 'aor' parameter as a filter only in once case - when AOR is added.
> > > 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.
> > > It's logging
> > > ast_debug(3, "Synchronizing endpoint '%s' with AORs '%s'\n",
> > > 		ast_sorcery_object_get_id(endpoint), endpoint->aors);
> > > even if does nothing
> > 
> > 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?
> 
> In real world how many endpoints have more then 1 AOR and how many endpoints have the same AOR?
> May be it'll be a little inefficient, but will prevent errors due to implicit coding.

If you'd like to see this you can leave your -1, I just won't get to it for a bit as I just put this up for review after a conversation on the community forum.


-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14056
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I3ee00023be2393113cd4e056599f23f3499ef164
Gerrit-Change-Number: 14056
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Tue, 31 Mar 2020 17:35:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200331/f1b5b904/attachment.html>