[Home]

Summary:ASTERISK-05297: [patch] pbx_builtin_setvar_helper and __ prefixed variables
Reporter:Manuel Guesdon (mguesdon)Labels:
Date Opened:2005-10-14 16:36:01Date Closed:2008-01-15 15:51:37.000-0600
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20051014__pbx_setvar.diff.txt
Description:Doing a pbx_builtin_setvar_helper(chan,variableName,NULL) with __ prefixed variable name doesn't remove variable value because pbx_builtin_setvar_helper() do a test on ast_var_name(variableName) whch return name without __ prefix.
Comments:By: Tilghman Lesher (tilghman) 2005-10-14 17:03:27

Patch uploaded
By: Manuel Guesdon (mguesdon) 2005-10-15 04:28:29

Thank you. It fix the problem.
By: Mark Spencer (markster) 2005-10-18 13:59:36

Actually this is wrong.  The existing behavior *is* correct, and they are totally separate variable names.
By: Tilghman Lesher (tilghman) 2005-10-18 14:14:54

I disagree.  If you run:

pbx_builtin_setvar_helper(chan, "foo", NULL);

foo becomes unset.  So it should be that if you run:

pbx_builtin_setvar_helper(chan, "__foo", NULL);

it should unset a variable; currently it does not.

The attached patch may not be the correct method; however, there is still a bug.
By: Tilghman Lesher (tilghman) 2005-10-18 14:43:17

It's worth noting that __foo and foo cannot effectively coexist.

This is because ast_var_name strips the __ when it returns the name.  This patch reconciles the two and makes the behavior sane.

Consider:
exten => 112,1,Set(foo=123)
exten => 112,n,Set(_foo=456)
exten => 112,n,Set(__foo=789)
exten => 112,n,NoOp(${foo} ${_foo} ${__foo})

results in:
   -- Executing Set("Zap/29-1", "foo=123") in new stack
   -- Executing Set("Zap/29-1", "_foo=456") in new stack
   -- Executing Set("Zap/29-1", "__foo=789") in new stack
   -- Executing NoOp("Zap/29-1", "789  ") in new stack

It should be clear from this result that they are, in fact, all the same variable name.


By: Kevin P. Fleming (kpfleming) 2005-10-18 15:11:56

(sorry for causing some confusion... obviously my understanding of how this was all currently working was a bit muddled and Mark reverted the patch at my request)

You are right, this is confusingly implemented at the moment. I can see the value of the patch, but I'm concerned about the invisible side-effects of having multiple names for the same variable. One option would be to make ast_var_name no longer behave the way it does, and the other would be this patch to make the 'set' operations not care about the prefixes at all.

I suppose if we update doc/README.variables to clearly document how these prefixes are completely ignored _except_ for during inheritance and that all references to the variables regardless of whether they are prefixed with one or two underscores result in the same variable being used, then that would take care of the confusion (along with this patch being committed again).

Corydon76, are you willing to update the patch to incorporate the documentation update?
By: Mark Spencer (markster) 2005-10-18 15:22:39

Least i can do is write the docs...  Fixed and documented in CVS head.
By: Digium Subversion (svnbot) 2008-01-15 15:51:37.000-0600

Repository: asterisk
Revision: 6818

U   trunk/pbx.c

------------------------------------------------------------------------
r6818 | markster | 2008-01-15 15:51:36 -0600 (Tue, 15 Jan 2008) | 2 lines

Be sure to not consider prefix on variable updates (bug ASTERISK-5297)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=6818