[asterisk-dev] [Code Review] res_agi: Review improvement for get_agi_cmd to differentiate between an error and an empty command list

[Matt Jordan]

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

Ship it!


Once the issue reporter confirms that this patch meets their intent.


/trunk/res/res_agi.c
<https://reviewboard.asterisk.org/r/2117/#comment13690>

    Typical nomenclature would be:
    if (res) {
     ...
    }
      



/trunk/res/res_agi.c
<https://reviewboard.asterisk.org/r/2117/#comment13691>

    You actually don't need this error message, since get_agi_cmd will already output an error message.


- Matt


On Sept. 18, 2012, 1:19 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2117/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2012, 1:19 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and Matt Jordan.
> 
> 
> Summary
> -------
> 
> This patch has a couple problems, but the aim is to improve the performance/responsiveness of launch_asyncagi when it calls the get_agi_cmd function. The reporter explained that prior to the patch an empty command list would result in a channel hanging for approximately 30 seconds with broken pipe errors.
> 
> There appear to be some problems with this patch which need to be worked out.
> 
> 
> This addresses bug ASTERISK-20109.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20109
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_agi.c 373118 
> 
> Diff: https://reviewboard.asterisk.org/r/2117/diff
> 
> 
> Testing
> -------
> 
> Untested on my end. From what the patcher had written and based on what I've looked over of the code so far, I'm guessing he had probably tested what happens when no agi commands are available, but not much else.
> 
> 
> Thanks,
> 
> jrose
> 
>

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