[asterisk-dev] [Code Review] Add IPv6 Address Support To Security Events Framework

[Simon Perreault]

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


I can't check the "ship it" checkbox because I don't know the event framework well enough. The IPv6 side of things looks good.


/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1777/#comment10366>

    Why is this new check necessary?



/trunk/main/manager.c
<https://reviewboard.asterisk.org/r/1777/#comment10367>

    This looks wrong. Why does a function named mansession_ast_sockaddr_remote() not take a struct mansession as parameter? Is it just a wrapper for ast_sockaddr_from_sin()? Is it still necessary?



/trunk/main/manager.c
<https://reviewboard.asterisk.org/r/1777/#comment10370>

    Would it be possible to remove this call to ast_sockaddr_from_sin()? I would expect that as modules become IPv6-ready such calls would disappear...



/trunk/main/security_events.c
<https://reviewboard.asterisk.org/r/1777/#comment10368>

    Please add parens around the or-condition to ensure correct operator precedence.



/trunk/main/security_events.c
<https://reviewboard.asterisk.org/r/1777/#comment10369>

    You could just make two calls to ast_str_append() to remove the need for temporary variables.


- Simon


On Feb. 29, 2012, 9:43 a.m., elguero wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1777/
> -----------------------------------------------------------
> 
> (Updated Feb. 29, 2012, 9:43 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The current API supports only IPv4 for security events. 
> 
> *  This patch changes the API to support IPv6 and updates the components that use this API.
> 
> *  It quiets a warning that is being displayed if binding to IPv6 and IPv4 ( udpbindaddr=:: ). 
> 
> *  It also eliminates an error that was being generated since the current implementation was treating an IPv6 socket address as if it was IPv4.
> 
> *  Some copyright dates were updated as well.
> 
> 
> This addresses bug ASTERISK-19447.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19447
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 357541 
>   /trunk/channels/chan_sip.c 357541 
>   /trunk/channels/sip/security_events.c 357541 
>   /trunk/include/asterisk/security_events_defs.h 357541 
>   /trunk/main/manager.c 357541 
>   /trunk/main/security_events.c 357541 
> 
> Diff: https://reviewboard.asterisk.org/r/1777/diff
> 
> 
> Testing
> -------
> 
> Tested SIP implementation on CentOS 5.7, connected with IPv4 clients and IPv6 clients.  Also tested SIP on Fedora 16 vm with IPv4.  Security log now shows IPv4 or IPv6 addresses when a security event is generated.
> 
> 
> Thanks,
> 
> elguero
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120229/d01e1d07/attachment.htm>