[asterisk-dev] [Code Review]: Pimp SIP Location

[Joshua Colp]

> On March 11, 2013, 10:52 a.m., Mark Michelson wrote:
> > /team/group/pimp_my_sip/channels/chan_gulp.c, lines 170-171
> > <https://reviewboard.asterisk.org/r/2379/diff/1/?file=34114#file34114line170>
> >
> >     Use RAII_VAR for these so you don't have to clean them up all over the place.

Changed.


> On March 11, 2013, 10:52 a.m., Mark Michelson wrote:
> > /team/group/pimp_my_sip/channels/chan_gulp.c, line 179
> > <https://reviewboard.asterisk.org/r/2379/diff/1/?file=34114#file34114line179>
> >
> >     s/a well/as well/

Changed.


> On March 11, 2013, 10:52 a.m., Mark Michelson wrote:
> > /team/group/pimp_my_sip/res/res_sip.c, line 262
> > <https://reviewboard.asterisk.org/r/2379/diff/1/?file=34117#file34117line262>
> >
> >     Make the local_uri variable static.

Changed.


> On March 11, 2013, 10:52 a.m., Mark Michelson wrote:
> > /team/group/pimp_my_sip/res/res_sip.c, line 266
> > <https://reviewboard.asterisk.org/r/2379/diff/1/?file=34117#file34117line266>
> >
> >     Make this static.

Changed.


> On March 11, 2013, 10:52 a.m., Mark Michelson wrote:
> > /team/group/pimp_my_sip/res/res_sip_session.c, lines 562-564
> > <https://reviewboard.asterisk.org/r/2379/diff/1/?file=34123#file34123line562>
> >
> >     Print a warning here.

Done.


> On March 11, 2013, 10:52 a.m., Mark Michelson wrote:
> > /team/group/pimp_my_sip/res/res_sip_endpoint_identifier_ip.c, lines 117-119
> > <https://reviewboard.asterisk.org/r/2379/diff/1/?file=34121#file34121line117>
> >
> >     Add a comment here explaining why the rule here is to "deny" the addresses we expect requests to arrive from. I'm guessing this is because there is an implicit "permit all" rule for ACLs and so adding deny rules allows you not to have false positives for matching?

Added.


> On March 11, 2013, 10:52 a.m., Mark Michelson wrote:
> > /team/group/pimp_my_sip/res/res_sip/location.c, lines 104-108
> > <https://reviewboard.asterisk.org/r/2379/diff/1/?file=34119#file34119line104>
> >
> >     Have you performed any tests where an endpoint name had a hyphen in it?

I've changed the separator to be something that would never appear in reality.


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2379/#review8024
-----------------------------------------------------------


On March 11, 2013, 7:09 a.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2379/
> -----------------------------------------------------------
> 
> (Updated March 11, 2013, 7:09 a.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Summary
> -------
> 
> Knowing where something is and how to dial it is quite important, so these changes implement the following to accomplish that:
> 
> 1. A low level API is now provided for location. It's a thin wrapper over sorcery allowing retrieval, creating, updating, and deleting of AORs/contacts. It also allows explicit contacts to be configured on the AOR itself.
> 2. A dialplan function (GULP_DIAL_CONTACTS) is now provided which creates a dial string capable of dialing all contacts on an AOR.
> 3. Dialing an endpoint, AOR, or SIP URI is now possible in dial strings.
> 4. res_sip_endpoint_identifier_ip is now fully configurable and can match IP ranges as well as individual IPs.
> 
> 
> Diffs
> -----
> 
>   /team/group/pimp_my_sip/include/asterisk/res_sip.h 382784 
>   /team/group/pimp_my_sip/channels/chan_gulp.c 382784 
>   /team/group/pimp_my_sip/include/asterisk/res_sip_session.h 382784 
>   /team/group/pimp_my_sip/res/res_sip.c 382784 
>   /team/group/pimp_my_sip/res/res_sip.exports.in 382784 
>   /team/group/pimp_my_sip/res/res_sip/location.c PRE-CREATION 
>   /team/group/pimp_my_sip/res/res_sip/sip_configuration.c 382784 
>   /team/group/pimp_my_sip/res/res_sip_endpoint_identifier_ip.c 382784 
>   /team/group/pimp_my_sip/res/res_sip_sdp_audio.c 382784 
>   /team/group/pimp_my_sip/res/res_sip_session.c 382784 
> 
> Diff: https://reviewboard.asterisk.org/r/2379/diff
> 
> 
> Testing
> -------
> 
> Tested outgoing calls galore in various combinations. Also tested incoming call matching with res_sip_endpoint_identifier_ip.
> 
> 
> Thanks,
> 
> Joshua
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130311/24b90d76/attachment-0001.htm>