[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/#review2704
-----------------------------------------------------------



/branches/1.4/channels/chan_local.c
<https://reviewboard.asterisk.org/r/903/#comment5857>

    Hm ... I wonder if it would make sense to change this to:
    
    if (write_info->version != AST_WHATEVER_VERSION) {
    
    so that the check is "if this structure is not the version I was compiled against".  What I had in mind for the version number is basically as an ABI version to detect if you have version skew anywhere.


- Russell


On 2010-09-10 11:59:00, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/903/
> -----------------------------------------------------------
> 
> (Updated 2010-09-10 11:59:00)
> 
> 
> Review request for Asterisk Developers and Russell Bryant.
> 
> 
> 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 285637 
>   /branches/1.4/funcs/func_channel.c 285637 
>   /branches/1.4/include/asterisk/channel.h 285637 
>   /branches/1.4/include/asterisk/frame.h 285637 
>   /branches/1.4/include/asterisk/pbx.h 285637 
> 
> 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
> 
>