[asterisk-dev] [svn-commits] tilghman: branch 1.4 r87120 - /branches/1.4/funcs/func_curl.c

[Tilghman Lesher]

On Friday 26 October 2007 20:10:52 Russell Bryant wrote:
> Tilghman Lesher wrote:
> > I believe you're mistaken.  I don't see any code in pbx.c where it is
> > assured that a channel cannot be NULL when calling functions.  I checked
> > both ast_func_read and pbx_substitute_variables_helper_full.
>
> This is the commit that I was referring to:
> :~/src/asterisk/1.4$ svn log -r 61765
>
> ------------------------------------------------------------------------
> r61765 | russell | 2007-04-23 13:17:00 -0500 (Mon, 23 Apr 2007) | 5 lines
>
> Some dialplan functions, such as CUT(), expect to operate on variables on a
> channel.  So, this little hack lets them work in places where a channel
> doesn't exist, such as within DUNDi configuration.
> (issue #9465, reported and patched by Corydon76, testing by blitzrage)
>
> ------------------------------------------------------------------------
>
> However, the code has since been changed such that it only creates the
> bogus channel in some circumstances, and not all of them.  In that case, I
> think there are some other dialplan functions that I made the same
> autoservice addition to that need the same fix.

Okay, but in any case, I think the better fix is not to run autoservice on
non-existent channels, not to do extra work.  In the case of CUT, it certainly
makes sense, because we need the channel for another purpose (that is,
for its variables).  However, with something like CURL, we don't need the
channel for any real purpose, so checking to see if the channel is NULL before
running autoservice would eliminate doing unnecessary work.

So, going a little further, here are all the functions that would benefit from
either approach:

ARRAY - Sets variables, but could just as easily create globals.  Autoservice
might be needed, though, if one of the variables set is a function.
Conditional autoservice.
REALTIME - Database operations could delay.  Conditional autoservice.
func_odbc - uses the channel to set parameters, so a channel should be
created, if NULL is passed.  Also, database access could be slow, so
autoservice should be added.
GROUP* - iffy.  However, since it depends upon a list lock that could get
lots of contention on a busy system, we should probably add autoservice.
However, since setting/checking GROUP on a nonexistent channel is
nonsensical, the autoservice should be conditional.
GLOBAL - Same as GROUP, add conditional autoservice (especially
since global variables don't need an associated channel).
STAT - If the file is located on a remote filesystem, there could be a delay
returning.  However, a channel is not strictly needed, so conditional
autoservice.
ENUMLOOKUP, TXTCIDNAME - Both are DNS lookups which may involve
delay, but neither really need a channel.  Conditional autoservice.
CUT - needs a channel, but no delay is possible.  No autoservice needed.
CURL - delay but channel not needed.  Conditional autoservice.

So in summary, in all but one case (func_odbc), we should add autoservice,
but conditionally, and in that last case, we need to ensure that the channel
exists, so autoservice should be added all the time.  Does this sound
reasonable?

-- 
Tilghman