Summary:	ASTERISK-05941: [patch] cleanup of parse_variable_name() and pbx_retrieve_variable()
Reporter:	Luigi Rizzo (rizzo)	Labels:
Date Opened:	2006-01-01 07:59:53.000-0600	Date Closed:	2006-01-09 14:23:44.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) pbx.diff ( 1) pbx2.diff
Description:	the attached patch documents and cleans up the code in the two functions above as follows (NO FUNCTIONAL CHANGES): parse_variable_name() + remove the inner switch and simplify the logic removing the need   for a goto and a couple of variables; pbx_retrieve_variable() + add extensive documentation in the code about the function internals; + rearrange some blocks to avoid repeated checks on variable 'c'; + use a new variable, 'found' to avoid the need for goto's; + use a new variable, 'need_substring', to avoid a recursive call; + document and make use of the default values for 's', the variable   holding the temporary return value, so that many checks for NULL,   or assignments of NULL are not needed anymore; + as above, calls to ast_copy_string() are replaced by only one instance   at the end; + a repeated block of code to search into chanvars is replaced by a for()   loop with proper arguments; The indentation of two external "if" statements has been set to 2 spaces to avoid large whitespace changes, which would obfuscate the patch. Proper indentation can be implemented at commit time, or in a separate commit to make things more clear. ****** ADDITIONAL INFORMATION ****** See the comment in the code about how headp is used. The code in the patch preserves the original behaviour, the '#if 0' block proposes an alternative. Please pick the preferred one, possibly updating the comment if it needs to.
Comments:	By: Tilghman Lesher (tilghman) 2006-01-08 09:35:38.000-0600 I'd prefer if you reversed the logic for variable found (i.e. assume not found and set in multiple places).  I know it's a bit more code, but it helps avoid future bugs.  Go ahead and do the re-indentation otherwise. By: Luigi Rizzo (rizzo) 2006-01-09 06:17:23.000-0600 updated according to corydon's suggestion, defaulting to not found. (As commented in detail in the code, s == &not_found means not found, other values including NULL mean found. This saves the use of an extra variable, and reduces the chance of forgetting to set it when new variables are added). The larger patch is because i have put in a proper indentation so it is ready for commit now.