[asterisk-dev] [Code Review]: chan_sip: Fix directmedia's use of ACL to limit remotebridging to certain host addresses for trunk

[jrose]

> On May 24, 2012, 10:32 a.m., opticron wrote:
> > /trunk/channels/chan_sip.c, line 30383
> > <https://reviewboard.asterisk.org/r/1924/diff/3/?file=28172#file28172line30383>
> >
> >     If p2_directmediaha is NULL, this will segfault.
> 
> Mark Michelson wrote:
>     It won't. It just will be a no-op.

This is false. The only thing p2_directmediaha is used for is as the argument ha to ast_apply_ha in apply_directmedia_ha, and a null argument to that won't cause a segfault.


> On May 24, 2012, 10:32 a.m., opticron wrote:
> > /trunk/channels/chan_sip.c, lines 30397-30403
> > <https://reviewboard.asterisk.org/r/1924/diff/3/?file=28172#file28172line30397>
> >
> >     This implies that either can be NULL.
> 
> Mark Michelson wrote:
>     It's fine for either or both to be NULL.

Yep, either can.


> On May 24, 2012, 10:32 a.m., opticron wrote:
> > /trunk/channels/chan_sip.c, line 30391
> > <https://reviewboard.asterisk.org/r/1924/diff/3/?file=28172#file28172line30391>
> >
> >     Ditto for p1_directmediaha.
> 
> Mark Michelson wrote:
>     And ditto here.

ditto.


- jrose


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


On May 21, 2012, 9:29 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1924/
> -----------------------------------------------------------
> 
> (Updated May 21, 2012, 9:29 a.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and Matt Jordan.
> 
> 
> Summary
> -------
> 
> Related to https://reviewboard.asterisk.org/r/1899/ which has some locking changes that need to be made still...
> 
> This patch takes Mark's suggested approach for adding callbacks for the rtp_bridge function to be able to supply two channels for determining rtp glue stuff in cases where a channel driver needs data about both peers in order to determine what types of bridging are permissible.
> 
> 
> This addresses bug AST-876.
>     https://issues.asterisk.org/jira/browse/AST-876
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 366775 
>   /trunk/include/asterisk/rtp_engine.h 366775 
>   /trunk/main/rtp_engine.c 366775 
> 
> Diff: https://reviewboard.asterisk.org/r/1924/diff
> 
> 
> Testing
> -------
> 
> Similar to the testing done for the review 1899 version.  Calls were tested with and without directmediapermit/deny on both sides of a call with calls being started from both directions. Specific things tested include whether or not the host address lists were copied properly (because that was a rather substantial problem earlier on) and the results of ast_apply_ha in each case.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120524/638d52a7/attachment-0001.htm>