[asterisk-dev] [Code Review] 4196: bridge_basic: Fix features issues introduced by review 4167

[rmudgett]

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



/branches/13/main/bridge_basic.c
<https://reviewboard.asterisk.org/r/4196/#comment24297>

    I think the test event needs to be moved to right after ast_app_dtget() with appropriate change to the res test for success/fail (0 < res ? "success" : "fail") rather than moving the memset to this awkward location.
    
    If the memset were done after the debug messages and before the retry_sound/invalid_sound playback you could then easily have those sounds interrupted by a digit to put into exten.


- rmudgett


On Nov. 19, 2014, 4:57 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4196/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2014, 4:57 p.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> In https://reviewboard.asterisk.org/r/4167, two bugs were introduced:
> 
> * Prior to entering the features retry loop, the buffer that holds the collected digits is wiped. However, this inadvertently wipes out the first collected digit on the first pass through, which is obtained in ast_stream_and_wait. This caused all of the features tests to fail.
> * If ast_app_dtget returns a hangup (-1), the loop would retry incorrectly. If we detect a hangup, we have to stop trying the feature.
> 
> This patch fixes both issues.
> 
> 
> Diffs
> -----
> 
>   /branches/13/main/bridge_basic.c 428298 
> 
> Diff: https://reviewboard.asterisk.org/r/4196/diff/
> 
> 
> Testing
> -------
> 
> Tested with the atxfer_nominal test. Prior to this patch it would fail; with this patch it completes successfully.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141119/f3b8b937/attachment-0001.html>