[asterisk-dev] [Code Review] New AMI Command LocalOptimizeAway

[Mark Michelson]

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



http://svn.asterisk.org/svn/asterisk/trunk/channels/chan_local.c
<https://reviewboard.asterisk.org/r/732/#comment4753>

    Style recommendation: Reverse the condition of the if statement like so:
    
    if (AST_LIST_LOCK(&locals)) {
        astman_send_error(blah);
        return 0;
    }
    
    Then there's no need to put the rest of the code in an else block, thus reducing the indentation level of the majority of the code in this function.



http://svn.asterisk.org/svn/asterisk/trunk/channels/chan_local.c
<https://reviewboard.asterisk.org/r/732/#comment4754>

    After the ast_mutex_unlock() call, you should add
    
    if (found) {
        break;
    }
    
    to reduce unnecessary looping.



http://svn.asterisk.org/svn/asterisk/trunk/channels/chan_local.c
<https://reviewboard.asterisk.org/r/732/#comment4755>

    I checked other manager commands, and it appears that we typically do a case-insensitive comparison for channel names. Use strcasecmp instead of strcmp here.


- Mark


On 2010-06-22 13:46:55, tim_ringenbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/732/
> -----------------------------------------------------------
> 
> (Updated 2010-06-22 13:46:55)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Adds a new manager command LocalOptimizeAway. It clears the flag on a local channel that says not to optimize it away.
> 
> General use case: You need the behavior of "/n" for a while, but then something else happens (typically a transfer) and you no longer desire that behavior.
> 
> Specific Use case: Using local channels as queue members without stateinterface, using /n to keep the local channel up so the agent doesn't get another call, then on transfer issuing this manager command so the local channel goes away and the agent can get another call.
> 
> 
> Diffs
> -----
> 
>   http://svn.asterisk.org/svn/asterisk/trunk/channels/chan_local.c 271972 
> 
> Diff: https://reviewboard.asterisk.org/r/732/diff
> 
> 
> Testing
> -------
> 
> We have the 1.4 version of the patch in production at several sites. The trunk version actually submitted here has only minimal testing: I logged into the manager once and issued the command and observed that it worked as expected.
> 
> 
> Thanks,
> 
> tim_ringenbach
> 
>