[asterisk-dev] [Code Review] IPv6 in Asterisk

[Mark Michelson]

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


This covers the rest of chan_sip. My afternoon is taken up by other obligations, so I hope to get to page 2 of the diff tomorrow.


/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/743/#comment5031>

    There are a few issues here.
    
    1. You must check the return value of ao2_alloc() to ensure it is non-NULL.
    
    2. Use of strtok is not threadsafe.
    
    3. If the proxyname for the peer is zero-length, you are setting the global outboundproxy's name to be zero-length, which is not correct.
    
    On the other hand, it looks like you fixed some reversed logic when parsing the "force" portion, so kudos!



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/743/#comment5032>

    Would ast_sockaddr_set_null() work here?



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/743/#comment5033>

    I realize after seeing this code here that your use of strtok in build_peer was largely based on its usage here. As such, I'd say not to worry too much about that comment since it's not something new you've introduced. Someone else can take a look at this as a separate piece of work.



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/743/#comment5034>

    I object to the removal of the bindport option, mainly with the argument that many configurations will be broken when upgrading to 1.8. If we want to discourage its use, then we can remove documentation of the feature from the sip.conf.sample file, but in the interest of backwards-compatibility, code to handle the option should remain.


- Mark


On 2010-06-30 09:31:29, Simon Perreault wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/743/
> -----------------------------------------------------------
> 
> (Updated 2010-06-30 09:31:29)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Summary
> -------
> 
> This is the port of Asterisk to IPv6.
> 
> 
> This addresses bug 17565.
>     https://issues.asterisk.org/view.php?id=17565
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/chan_ooh323.c 273189 
>   /trunk/apps/app_externalivr.c 273189 
>   /trunk/channels/chan_gtalk.c 273189 
>   /trunk/channels/chan_h323.c 273189 
>   /trunk/channels/chan_iax2.c 273189 
>   /trunk/channels/chan_jingle.c 273189 
>   /trunk/channels/chan_mgcp.c 273189 
>   /trunk/channels/chan_multicast_rtp.c 273189 
>   /trunk/channels/chan_sip.c 273189 
>   /trunk/channels/chan_skinny.c 273189 
>   /trunk/channels/chan_unistim.c 273189 
>   /trunk/channels/sip/dialplan_functions.c 273189 
>   /trunk/channels/sip/include/dialog.h 273189 
>   /trunk/channels/sip/include/globals.h 273189 
>   /trunk/channels/sip/include/reqresp_parser.h 273189 
>   /trunk/channels/sip/include/sip.h 273189 
>   /trunk/channels/sip/reqresp_parser.c 273189 
>   /trunk/include/asterisk/acl.h 273189 
>   /trunk/include/asterisk/config.h 273189 
>   /trunk/include/asterisk/dnsmgr.h 273189 
>   /trunk/include/asterisk/netsock2.h PRE-CREATION 
>   /trunk/include/asterisk/rtp_engine.h 273189 
>   /trunk/include/asterisk/tcptls.h 273189 
>   /trunk/main/acl.c 273189 
>   /trunk/main/app.c 273189 
>   /trunk/main/config.c 273189 
>   /trunk/main/dnsmgr.c 273189 
>   /trunk/main/http.c 273189 
>   /trunk/main/manager.c 273189 
>   /trunk/main/netsock2.c PRE-CREATION 
>   /trunk/main/rtp_engine.c 273189 
>   /trunk/main/tcptls.c 273189 
>   /trunk/res/res_rtp_asterisk.c 273189 
>   /trunk/res/res_rtp_multicast.c 273189 
> 
> Diff: https://reviewboard.asterisk.org/r/743/diff
> 
> 
> Testing
> -------
> 
> See test report on the mantis issue.
> 
> 
> Thanks,
> 
> Simon
> 
>