[asterisk-dev] [Code Review] Add Path header support to chan_sip

[Olle E Johansson]

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


A first set of comments from a hotel in Germany.


/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/991/#comment6177>

    Or correctly spelled. "Updating". It should be "ast_debug(2," instead of ast_log(LOG_DEBUG



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/991/#comment6178>

    Try to make it "Path header" instead. "Use path" doesn't mean much to people but if you refer to a header, they can guess a bit more.



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/991/#comment6179>

    Just check the reset of the code in there. It's not hard.
    
    I would prefer "pathsupport" or something else than "use"...



/branches/1.8/configs/sip.conf.sample
<https://reviewboard.asterisk.org/r/991/#comment6180>

    It's not "router" header, it's "Route: header"


- Olle E


On 2010-10-29 13:46:27, Klaus Darilion wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/991/
> -----------------------------------------------------------
> 
> (Updated 2010-10-29 13:46:27)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch adds support for the Parse header (RFC 3327). This allows to record the Path (route) of incoming REGISTER requests and then reuse the learned path on outgoing out-of-dialog requests as a pre-loaded route set.
> 
> I tried to reuse as much of existing code as possible. There are several parts where I am not sure if it is implemented nice and follow chan_sip design rules (are there any?) I will post my questions as a review to myself.
> 
> 
> This addresses bug 18223.
>     https://issues.asterisk.org/view.php?id=18223
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 293272 
>   /branches/1.8/channels/sip/include/sip.h 293272 
>   /branches/1.8/configs/sip.conf.sample 293272 
> 
> Diff: https://reviewboard.asterisk.org/r/991/diff
> 
> 
> Testing
> -------
> 
> Basic testing done (with nat=yes|no, with and without outboundproxy set for a peer, with and without Parse header, single Parse header and two Parse headers, restarting Asterisk)
> 
> 
> Thanks,
> 
> Klaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.digium.com/pipermail/asterisk-dev/attachments/20101119/1a72b627/attachment-0001.htm