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

Simon Perreault simon.perreault at viagenie.ca
Mon Jul 5 10:04:41 CDT 2010 


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 13434-13441
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11249#file11249line13434>
> >
> >     Missed this last time around, change in behaviour.  Port has been removed.
> 
> Mark Michelson wrote:
>     Port is still present here, but you're correct for the other manager events you mention. One of the common changes in this diff is to append the port to the address in many cases. For manager events, though, I agree that we should still include the port as a separate header for ease of upgrade.
> 
> pabelanger wrote:
>     My mistake, it was the next manager events that had port dropped :)

I'll just put Port on a separate line for readability.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 13455-13457
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11249#file11249line13455>
> >
> >     same comment

Fixed in r273977.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 13469-13471
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11249#file11249line13469>
> >
> >     same comment

Fixed in r273977.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 13481-13484
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11249#file11249line13481>
> >
> >     same comment

Fixed in r273977.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 16283-16284
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11249#file11249line16283>
> >
> >     braces since we are here.

Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/include/asterisk/acl.h, lines 145-146
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11258#file11258line145>
> >
> >     blobs

Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/include/asterisk/dnsmgr.h, line 53
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11260#file11260line53>
> >
> >     This comment caught my eye.  I don't think we want to removed it, but would it not be worth the time an effort to start enforcing \version tag in doxygen comments?
> >     
> >     A general questions.
> 
> Mark Michelson wrote:
>     For some reason, Review Board ate this comment when I published my last reply.
>     
>     I like the idea of encouraging the use of \version for API changes. For functions added in a particular version, we typically use the \since tag instead.

Re-added the comment and added

\version 1.8.0 result changed from struct ast_sockaddr_in to ast_sockaddr for IPv6 support

in r273978.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/main/acl.c, lines 399-400
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11264#file11264line399>
> >
> >     braces

Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/main/acl.c, lines 408-409
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11264#file11264line408>
> >
> >     same

Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/main/acl.c, lines 555-556
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11264#file11264line555>
> >
> >     same

Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/main/acl.c, lines 568-569
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11264#file11264line568>
> >
> >     same

Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/main/config.c, lines 2394-2395
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11266#file11266line2394>
> >
> >     braces

Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/main/netsock2.c, lines 130-135
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11270#file11270line130>
> >
> >     braces
> 
> Mark Michelson wrote:
>     This one is particularly bad since the braces on the first if are not matched, leading to a compiler error. I can't believe I missed this :)

Wow, super cool. Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/main/netsock2.c, lines 144-145
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11270#file11270line144>
> >
> >     same

Fixed.


> On 2010-07-02 08:05:18, pabelanger wrote:
> > /trunk/res/res_rtp_asterisk.c, lines 2380-2382
> > <https://reviewboard.asterisk.org/r/743/diff/4/?file=11273#file11273line2380>
> >
> >     braces

Fixed.


- Simon


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


On 2010-07-01 09:30:54, Simon Perreault wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/743/
> -----------------------------------------------------------
> 
> (Updated 2010-07-01 09:30:54)
> 
> 
> 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 273198 
>   /trunk/apps/app_externalivr.c 273198 
>   /trunk/channels/chan_gtalk.c 273198 
>   /trunk/channels/chan_h323.c 273198 
>   /trunk/channels/chan_iax2.c 273198 
>   /trunk/channels/chan_jingle.c 273198 
>   /trunk/channels/chan_mgcp.c 273198 
>   /trunk/channels/chan_multicast_rtp.c 273198 
>   /trunk/channels/chan_sip.c 273198 
>   /trunk/channels/chan_skinny.c 273198 
>   /trunk/channels/chan_unistim.c 273198 
>   /trunk/channels/sip/dialplan_functions.c 273198 
>   /trunk/channels/sip/include/dialog.h 273198 
>   /trunk/channels/sip/include/globals.h 273198 
>   /trunk/channels/sip/include/reqresp_parser.h 273198 
>   /trunk/channels/sip/include/sip.h 273198 
>   /trunk/channels/sip/reqresp_parser.c 273198 
>   /trunk/include/asterisk/acl.h 273198 
>   /trunk/include/asterisk/config.h 273198 
>   /trunk/include/asterisk/dnsmgr.h 273198 
>   /trunk/include/asterisk/netsock2.h PRE-CREATION 
>   /trunk/include/asterisk/rtp_engine.h 273198 
>   /trunk/include/asterisk/tcptls.h 273198 
>   /trunk/main/acl.c 273198 
>   /trunk/main/app.c 273198 
>   /trunk/main/config.c 273198 
>   /trunk/main/dnsmgr.c 273198 
>   /trunk/main/http.c 273198 
>   /trunk/main/manager.c 273198 
>   /trunk/main/netsock2.c PRE-CREATION 
>   /trunk/main/rtp_engine.c 273198 
>   /trunk/main/tcptls.c 273198 
>   /trunk/res/res_rtp_asterisk.c 273198 
>   /trunk/res/res_rtp_multicast.c 273198 
> 
> Diff: https://reviewboard.asterisk.org/r/743/diff
> 
> 
> Testing
> -------
> 
> See test report on the mantis issue.
> 
> 
> Thanks,
> 
> Simon
> 
>

More information about the asterisk-dev mailing list