[asterisk-dev] Re: [svn-commits] murf: trunk r63046 - in /trunk: ./ channels/ include/asterisk/ res/

[Russell Bryant]

----- svn-commits at lists.digium.com wrote:
> Author: murf
> Date: Fri May  4 11:37:23 2007
> New Revision: 63046
> 
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=63046
> Log:
> Added a small bit of code to support the SNOM 360's Record button.
> Made the find_feature func in res_features.c public, so I could use it
> to find the automon dial sequence as configured by the user. When the
> INFO packet has a Record: header with on/off, the sequence is sent as
> consecutive DTMF frames on the phone's channel, triggering the automon
> functionality. The user has to configure the automon in features.conf,
> and set up his dialplan accordingly.

A couple of comments:

1) If this function is going to be public, please add an ast_ prefix to the function name.

2) There is no synchronization between res_features and other modules that may be using find_feature() to look at feature information.  There needs to be an ast_(un)lock_features(), or the function should just return a copy, and then have a function that can destroy the copy.

Also, I would prefer that this function return a const pointer, to ensure that external modules don't try to modify them.

-- 
Russell Bryant
Software Engineer
Digium, Inc.