[asterisk-dev] [Code Review] Allow disconnect feature before a call is bridged

David Vossel dvossel at digium.com
Wed Mar 18 18:08:50 CDT 2009


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

(Updated 2009-03-18 18:08:50.134028)


Review request for Asterisk Developers and Russell Bryant.


Changes
-------

ffffffffffffffffffixed maybe


Summary
-------

res_feature's main procedure is called within ast_bridge_call().  This means features such as the disconnect action are only available after the bridge is established.  The goal of this patch is to use the user configured feature code during the call setup phase.

This patch was created by a community member, sobomax, and originally assigned to someone else before being assigned to me.  I've taken what I believe was latest code and cleaned up what I could.  There were quite a few instances of string allocation taking place on the heap where they could have taken place on the stack.  I believe I caught them all, but that would be something to look for.  I also had some concerns about the way locking was done.  In detect_disconnect() the channel wasn't locked when I believe it should have been.  For a moment, features and the channel must be locked at the same time.  I don't suspect this to a problem, but it would be nice if someone could verify my locking order.  Other than that, my main concern is not fully understanding the implications of this code.  I understand what it does, but not everything it could affect. 


This addresses bug 0011583.
    http://bugs.digium.com/view.php?id=0011583


Diffs (updated)
-----

  /trunk/apps/app_dial.c 183032 
  /trunk/include/asterisk/features.h 183032 
  /trunk/main/features.c 183032 

Diff: http://reviewboard.digium.com/r/195/diff


Testing
-------

I've tested the patch using the disconnect feature. Made a call and was able to successfully disconnect before bridging. 


Thanks,

David




More information about the asterisk-dev mailing list