[asterisk-dev] [Code Review] 2642: app_bridgewait: Add a name argument so that multiple holding bridges may be used

[rmudgett]

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



/trunk/apps/app_bridgewait.c
<https://reviewboard.asterisk.org/r/2642/#comment18184>

    Change app_bridgewait to <literal>BridgeWait</literal>



/trunk/apps/app_bridgewait.c
<https://reviewboard.asterisk.org/r/2642/#comment18183>

    Spurious change.



/trunk/apps/app_bridgewait.c
<https://reviewboard.asterisk.org/r/2642/#comment18191>

    This line is too long.
    Do:
    bridge_wrapper = alloc()
    if (!bridge_wrapper)
    



/trunk/apps/app_bridgewait.c
<https://reviewboard.asterisk.org/r/2642/#comment18192>

    This can just be:
    /* The bridge reference is unconditionally passed. */
    return wait_bridge_wrapper_alloc();



/trunk/apps/app_bridgewait.c
<https://reviewboard.asterisk.org/r/2642/#comment18193>

    Scope the lock around this code rather than the whole function.  Actually you don't need to use scoped locking here as it is overkill.



/trunk/apps/app_bridgewait.c
<https://reviewboard.asterisk.org/r/2642/#comment18194>

    Since you only need this container to reject duplicates, you could change this to a hash container with sorted buckets.  This way if there are a lot of holding bridges the performance shouldn't degrade as badly as a simple list.


- rmudgett


On July 25, 2013, 7:07 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2642/
> -----------------------------------------------------------
> 
> (Updated July 25, 2013, 7:07 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-21922
>     https://issues.asterisk.org/jira/browse/ASTERISK-21922
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Previously the holding bridge used for bridgewait was a single bridge that once created would stick around until the application was unloaded.
> 
> Now things are a little more complicated. Each holding bridge has a name and will be destroyed when it is empty. Since it's theoretically possible to have channels enter a holding bridge without going through the bridge wait application, it isn't always possible to deliberately control when the bridge will be destroyed, so the bridge can effectively be abandoned and set to be destroyed once it empties out.
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_bridgewait.c 395167 
>   /trunk/include/asterisk/bridge.h 395423 
>   /trunk/include/asterisk/bridge_channel_internal.h 395423 
>   /trunk/main/bridge.c 395423 
>   /trunk/main/bridge_channel.c 395423 
> 
> Diff: https://reviewboard.asterisk.org/r/2642/diff/
> 
> 
> Testing
> -------
> 
> Tested multiple simultaneous holding bridges, multiple channels in the same holding bridge, what would happen if one channel entered as another one was leaving but before the bridge was destroyed, tested bridges emptying out, tested module unload and module load after unload.
> 
> 
> Thanks,
> 
> jrose
> 
>

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