[asterisk-dev] [Code Review] Allow inheritance of CHANNEL() write changes for Local/n queue members

[Russell Bryant]

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



/branches/1.4/funcs/func_channel.c
<https://reviewboard.asterisk.org/r/903/#comment5831>

    I think you need to initialize loop to 0.  I also wonder if there is a clearer way to write this.  Maybe ...
    
    unsigned int loop_iterations = 2;
    
    do {
    
    } while (--loop_iterations && .. )



/branches/1.4/include/asterisk/channel.h
<https://reviewboard.asterisk.org/r/903/#comment5832>

    I haven't been able to think of a problem of the approach in general.  However, I really don't think it's wise to change the ast_channel_tech API in released versions.  You could probably abuse the getoption() callback to accomplish what you need.
    
    However, a bigger implementation concern is the safety of reaching over to the other side of the Local channel.  To guarantee that channel is valid, you'll need some crazy locking.
    
    You would need to ensure that get_other_side is called without a channel lock held.  In the the callback, you need to lock local_pvt.  Finally, you will need to lock the other side channel before returning it, which will involve some deadlock avoidance.
    
    :-(


- Russell


On 2010-09-03 09:05:00, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/903/
> -----------------------------------------------------------
> 
> (Updated 2010-09-03 09:05:00)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Having Local (/n) channels as queue members and setting the language in the extension with Set(CHANNEL(language)=fr) sets the language on the Local/...,2 channel. Hold time report playbacks happen on the Local/...,1 channel and therefor do not play in the specified language.
> 
> This patch adds a channel callback function get_other_side() (I am totally open to another name for this) that given one side of a local channel, will pass back the other side. This is used in the func_channel_write function to apply any changes to both sides of the local channel.
> 
> There are other places in the code where we specifically check for a local channel and look up the other side of the channel via relying on the names of the channels being similar and doing an ast_get_channel_by_name_locked(), but that is just exceedingly gross. We can't use the get_base_channel() callback because that could have some unintended consequences during a masquerade.
> 
> I propose that we make this change in all supported release versions, despite it being an API change, since there really is no other way for a local (/n) channel in a queue to set the desired language without *some* kind of fix--and this is the least hacky way I could come up with.
> 
> 
> This addresses bug 17673.
>     https://issues.asterisk.org/view.php?id=17673
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/channels/chan_local.c 284782 
>   /branches/1.4/funcs/func_channel.c 284782 
>   /branches/1.4/include/asterisk/channel.h 284782 
> 
> Diff: https://reviewboard.asterisk.org/r/903/diff
> 
> 
> Testing
> -------
> 
> ;queues.conf
> [test]
> strategy=ringall
> member => Local/s at queuetest/n
> reportholdtime=yes
> 
> ;extensions.conf
> [sip]
> exten => 123,1,Queue(test)
> 
> [queuetest]
> exten => s,1,Set(CHANNEL(language)=fr)
> exten => s,n,Dial(SIP/6004)
> 
> The hold time notification played in french.
> 
> 
> Thanks,
> 
> Terry
> 
>