[asterisk-dev] [Code Review] 2633: Filter channels used as internal mechanisms

[Joshua Colp]

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


I'm not a huge fan of this approach. It feels unclean and hackish to use the role for determining whether to filter things. I think I would prefer having both a channel flag and this callback. You get the informational aspect of the role information, but also an explicit ability to filter a channel and they each have their clearly defined purpose.

- Joshua Colp


On June 21, 2013, 12:56 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2633/
> -----------------------------------------------------------
> 
> (Updated June 21, 2013, 12:56 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21873
>     https://issues.asterisk.org/jira/browse/ASTERISK-21873
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This adds a new function pointer to the channel tech which determines whether a channel is fit for external consumption. Examples of channels that would not be fit for external consumption include the announcement and recording channels used by confbridge which are the only two marked as such by this patch.
> 
> A lot of this ended up being cleanup to ensure callers of various functions could handle a NULL return.
> 
> 
> Diffs
> -----
> 
>   trunk/apps/confbridge/conf_chan_announce.c 392407 
>   trunk/apps/confbridge/conf_chan_record.c 392407 
>   trunk/apps/confbridge/confbridge_manager.c 392407 
>   trunk/include/asterisk/channel.h 392407 
>   trunk/include/asterisk/stasis_channels.h 392407 
>   trunk/main/cdr.c 392407 
>   trunk/main/cel.c 392407 
>   trunk/main/manager_bridging.c 392407 
>   trunk/main/manager_channels.c 392407 
>   trunk/main/stasis_channels.c 392407 
>   trunk/res/parking/parking_manager.c 392407 
> 
> Diff: https://reviewboard.asterisk.org/r/2633/diff/
> 
> 
> Testing
> -------
> 
> Ran through CDR unit tests.
> 
> 
> Thanks,
> 
> opticron
> 
>

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