[asterisk-dev] bweschke: branch 1.4 r150816 - /branches/1.4/main/manager.c

[BJ Weschke]

Kevin P. Fleming wrote:
> BJ Weschke wrote:
>
>   
>>  With the current patch, we only take on the hit on performance on the 
>> execution of a manager GetVar action when it's determined that the 
>> varname is likely a function. If we move this logic into the guts of 
>> ast_func_read proper, isn't that a much larger creep on the scope of 
>> that performance hit?
>>     
>
> I believe Russell's proposal was to just modify the dialplan functions
> themselves to safely abort if no channel is present; this is a simple
> check for the channel pointer being NULL or non-NULL, so it's quick and
> easy. This would avoid *ever* needing to create a dummy channel for the
> function to use.
>
>   
 We could definitely do this. However, that would involve an audit of all dialplan functions to see whether the check is appropriate or not.

 In addition, we'd also need to make corrections where folks assumed that the channel variable is always there, but isn't necessarily "core" to the dialplan functionality itself. 

 eg - from GROUP_COUNT functionality...

        if ((count = ast_app_group_get_count(group, category)) == -1) {
                ast_log(LOG_NOTICE, "No group could be found for channel '%s'\n", chan->name);
        }


 With all of this complete and a coding guideline in place to prevent re-occurence, we could drop the requirement for checking and the creation of the bogus channel in two different places of the code, but that work would be non-trivial and probably is cause for something like a janitor project, no? 
 
 In the mean time, this quick little fix eliminates the potential for a crash that currently exists when AMI is used incorrectly.
 
--
Bird's The Word Technologies, Inc.
http://www.btwtech.com/