Summary:ASTERISK-07160: [patch] ast_func_read segfault
Reporter:Curt Moore (jcmoore)Labels:
Date Opened:2006-06-13 11:46:41Date Closed:2011-06-07 14:00:44
Versions:Frequency of
Environment:Attachments:( 0) pbx_backtrace.txt
( 1) pbx_patch_v2.diff
( 2) pbx_patch.diff
Description:In pbx.c, when ast_func_read passes the "function" variable to the func_args function, func_args modifies the value in the "function" variable in addtion to retuning the "args" variable.  We should make a copy of the "function" variable and pass this copy to func_args instead.  

This appears to have happened as a result of splitting ast_func_read into 2 functions as it was previously in the 1.2 branch.
Comments:By: Andrey S Pankov (casper) 2006-06-13 12:45:29

Please look at the code in trunk and coding guidelines...
- do not declare vars mid-block
- use ast_strlen_zero
- use ast_strdup (ast_strdupa maybe?)
- formatting!!!

By: Jon Hood (squinky86) 2006-06-13 13:49:20

casper, I don't think he is declaring vars mid-block in a confusing way. It is better to go ahead and return from a function if you can before initializing the variables. It takes up less time and memory. This is just my opinion and has no bearing on what the asterisk devs want to do.

By: Curt Moore (jcmoore) 2006-06-13 13:54:25

All of the proposed suggestions have been made in pbx_patch_v2.diff.  Please remove pbx_patch.diff.

By: Serge Vecher (serge-v) 2006-06-13 14:17:20

under which conditions would a segfault occur? Do you have a backtrace for it?

By: Curt Moore (jcmoore) 2006-06-13 14:45:58

The segfault would occur any time I tried to use the function.  An example code snippet:

char cidname_ws[2048];
ast_func_read(chan, "CALLERID(name)", cidname_ws, sizeof(cidname_ws));

I could provide a backtrace but it would likely be of little use since I'm calling ast_func_read from some other code outside of the Asterisk codebase.

By: Tilghman Lesher (tilghman) 2006-06-13 21:57:05

We're going to need to see that backtrace and the associated source code.  Given that this function is working as documented, the problem would appear to be in your code.

By: Curt Moore (jcmoore) 2006-06-13 22:54:22

I attached a somewhat sanitized backtrace. Unfortunately, I'm not going to be able to provide my full source code which calls ast_func_read, but it is essentially the following:

char cidnum_ws[2048];
ast_func_read(chan, "CALLERID(num)", cidnum_ws, sizeof(cidnum_ws));

If the consensus is that pbx.c is correct, I can live with that but I've been running the exact same external code on the 1.0 and 1.2 codebase for the past 8-10 months without issue.  As mentioned previously, this only happened when the argument parsing portion of ast_func_read was split out into the func_args function in the trunk/1.4 codebase.

I wish I could provide more debug info so we could continue to discuss but it's just not going to be possible.  If there's anything else I can do to try and debug this further, let me know. Thanks for the help and sorry for the trouble.

By: Curt Moore (jcmoore) 2006-06-14 10:06:55

If everyone agrees, let's go ahead and close this. I'll just dynamically allocate the function variable "CALLERID(num)" in my code instead of passing it in as a static string to ast_func_read, which is what's causing the issue. Always more than 1 way to do things. :-)

By: Tilghman Lesher (tilghman) 2006-06-14 14:39:11

3 items of interest before this gets closed:

1)  There's a much easier interface to use to get callerID in the Asterisk API, specifically by using the channel structure to access the fields directly.

2)  You could also call pbx_substitute_variables_helper() with a parameter of "${CALLERID(name)}".

3)  I hope you realize that your code is covered under the GPL, as it links with Asterisk, unless you've obtained a special arrangement with Digium to distribute Asterisk under a different license.  Hence, obscuring your backtrace has no useful purpose, other than simply to annoy other users of the bugtracker.