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

Tue Jun 29 08:24:14 CDT 2010

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 16957-16962
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line16957>
> >
> >     spacing between if and (, braces for if statements.

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 17299-17310
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line17299>
> >
> >     Braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 23652-23653
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line23652>
> >
> >     missing braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 23683-23684
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line23683>
> >
> >     braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 24765-24766
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line24765>
> >
> >     spacing and braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, line 26028
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26028>
> >
> >     blob

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, line 26133
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26133>
> >
> >     Hard-coded setting?

This is a default setting. It is hard-coded, like all others. (How could defaults be soft-coded?)

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, line 26345
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26345>
> >
> >     spacing

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 26350-26352
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26350>
> >
> >     braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 26520-26521
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26520>
> >
> >     again, braces.

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 26546-26548
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26546>
> >
> >     spacing and braces.

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 26913-26916
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26913>
> >
> >     braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 26937-26938
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26937>
> >
> >     Braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 26969-26972
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26969>
> >
> >     Braces, and red blob

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 26975-26979
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26975>
> >
> >     same comment

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 26982-26983
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line26982>
> >
> >     same

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 27657-27658
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line27657>
> >
> >     Braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, line 27690
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line27690>
> >
> >     blob

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 27699-27700
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line27699>
> >
> >     braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, line 27704
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line27704>
> >
> >     remove space before ;

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, lines 27713-27714
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line27713>
> >
> >     braces

Fixed.

> On 2010-06-28 15:37:42, pabelanger wrote:
> > /trunk/channels/chan_sip.c, line 27649
> > <https://reviewboard.asterisk.org/r/743/diff/3/?file=11155#file11155line27649>
> >
> >     Does temporary still apply after porting?

This comment was misleading. It was changed changed to the following:

/*! \brief  Return the first entry from ast_sockaddr_resolve.
 *
 * \warn Using this function probably means you have a faulty design.
 */

(The fact this this function is used throughout chan_sip highlights a problem that is orthogonal to IPv6 support.)

- Simon

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

On 2010-06-28 14:29:33, Simon Perreault wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/743/
> -----------------------------------------------------------
> 
> (Updated 2010-06-28 14:29:33)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Summary
> -------
> 
> This is the port of Asterisk to IPv6.
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/chan_ooh323.c 272684 
>   /trunk/apps/app_externalivr.c 272684 
>   /trunk/channels/chan_gtalk.c 272684 
>   /trunk/channels/chan_h323.c 272684 
>   /trunk/channels/chan_iax2.c 272684 
>   /trunk/channels/chan_jingle.c 272684 
>   /trunk/channels/chan_mgcp.c 272684 
>   /trunk/channels/chan_multicast_rtp.c 272684 
>   /trunk/channels/chan_sip.c 272684 
>   /trunk/channels/chan_skinny.c 272684 
>   /trunk/channels/chan_unistim.c 272684 
>   /trunk/channels/sip/dialplan_functions.c 272684 
>   /trunk/channels/sip/include/dialog.h 272684 
>   /trunk/channels/sip/include/globals.h 272684 
>   /trunk/channels/sip/include/reqresp_parser.h 272684 
>   /trunk/channels/sip/include/sip.h 272684 
>   /trunk/channels/sip/reqresp_parser.c 272684 
>   /trunk/include/asterisk/acl.h 272684 
>   /trunk/include/asterisk/config.h 272684 
>   /trunk/include/asterisk/dnsmgr.h 272684 
>   /trunk/include/asterisk/netsock2.h PRE-CREATION 
>   /trunk/include/asterisk/rtp_engine.h 272684 
>   /trunk/include/asterisk/tcptls.h 272684 
>   /trunk/main/acl.c 272684 
>   /trunk/main/app.c 272684 
>   /trunk/main/config.c 272684 
>   /trunk/main/dnsmgr.c 272684 
>   /trunk/main/http.c 272684 
>   /trunk/main/manager.c 272684 
>   /trunk/main/netsock2.c PRE-CREATION 
>   /trunk/main/rtp_engine.c 272684 
>   /trunk/main/tcptls.c 272684 
>   /trunk/res/res_rtp_asterisk.c 272684 
>   /trunk/res/res_rtp_multicast.c 272684 
> 
> Diff: https://reviewboard.asterisk.org/r/743/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Simon
> 
>