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-0600Date Closed:2006-01-09 14:23:44.000-0600
Versions:Frequency of
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):

+ remove the inner switch and simplify the logic removing the need
  for a goto and a couple of variables;

+ 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.


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.