[asterisk-dev] [Code Review] make disconnect feature code available outside of bridge

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/186/#review521
-----------------------------------------------------------



/trunk/apps/app_dial.c
<http://reviewboard.digium.com/r/186/#comment1221>

    This is not thread-safe.  The result from this function is only valid as while the channel is locked.



/trunk/apps/app_dial.c
<http://reviewboard.digium.com/r/186/#comment1219>

    with the way you have featurecode defined, how is this condition possible?



/trunk/apps/app_dial.c
<http://reviewboard.digium.com/r/186/#comment1220>

    The use of ast_str would simplify this code.  This becomes a simple ast_str_append() call.



/trunk/apps/app_dial.c
<http://reviewboard.digium.com/r/186/#comment1218>

    stray whitespace



/trunk/include/asterisk/features.h
<http://reviewboard.digium.com/r/186/#comment1222>

    The whitespace here was right before it got moved



/trunk/include/asterisk/features.h
<http://reviewboard.digium.com/r/186/#comment1223>

    I thought we removed KEEPALIVE?



/trunk/include/asterisk/features.h
<http://reviewboard.digium.com/r/186/#comment1224>

    This is an entry in the public API.  It must have an ast_ prefix.



/trunk/include/asterisk/features.h
<http://reviewboard.digium.com/r/186/#comment1225>

    Where does the magic 20 come from?



/trunk/include/asterisk/features.h
<http://reviewboard.digium.com/r/186/#comment1226>

    Please include documentation with a new API call.



/trunk/main/features.c
<http://reviewboard.digium.com/r/186/#comment1227>

    more stray whitespace (I won't mark the rest)



/trunk/main/features.c
<http://reviewboard.digium.com/r/186/#comment1228>

    1) You should be using ast_strdup() instead of strdup().
    
    2) You should be checking for memory allocation failure.
    
    However, since this is local to the function, you don't need mallocd memory, anyway.  You can just use ast_strdupa().


- Russell


On 2009-03-05 10:59:28, Steve Murphy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/186/
> -----------------------------------------------------------
> 
> (Updated 2009-03-05 10:59:28)
> 
> 
> Review request for Asterisk Developers and Russell Bryant.
> 
> 
> Summary
> -------
> 
> This is an updated patch for bug 11583. I have fixed (I believe) all problems that Russell
> pointed out on 2 Feb 2009, 11:44 am in asterisk-dev.
> 
> Basically, the user who submitted this patch wants to make the disconnect feature
> be recognized during the dialing process. They rearranged some of the feature code
> into an exported routine, and called this from the dial app while waiting for an answer.
> 
> 
> This addresses bug 11583.
>     http://bugs.digium.com/view.php?id=11583
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 180282 
>   /trunk/apps/app_dial.c 180282 
>   /trunk/include/asterisk/features.h 180282 
>   /trunk/main/features.c 180282 
> 
> Diff: http://reviewboard.digium.com/r/186/diff
> 
> 
> Testing
> -------
> 
> I have tested the functionality on my test machine; it is working.
> 
> 
> Thanks,
> 
> Steve
> 
>