Summary:	ASTERISK-04045: [patch] SetVar paradox
Reporter:	Anthony Minessale (anthm)	Labels:
Date Opened:	2005-05-03 12:30:57	Date Closed:	2008-01-15 15:33:15.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) setvar_rev0.diff
Description:	This patch fixes a bug that was added when my first patch was modified upon acceptance. The code allowed you to set options as the last arguement but all of the options are required to be set before you set any vars so with this patch it's moved to the first option rather than the last in fact you can set flags in any position in the list so you can do different combos inline also the R flag which really does nothing allows you to clear all the flags set sofar as any new flag string will preempt any previous one SetVar(c|example1=true|cg|example2=true|R|example3=true) example1 is a channel and a cdr var example2 is a cdr and a global var example3 is just a channel var ****** ADDITIONAL INFORMATION ****** Disclaimer on file anthmct@yahoo.com
Comments:	By: Kevin P. Fleming (kpfleming) 2005-05-03 20:17:57 I don't see anything in the patch to handle the 'R' flag, in fact it appears to clear out the previously specified flags for each variable, the 'R' flag wouldn't be needed at all. Is this the correct patch? By: Anthony Minessale (anthm) 2005-05-03 22:26:42 yah, any new flag string being clears all the flags. I just documented R for comfort. really anything clears it. By: Kevin P. Fleming (kpfleming) 2005-05-04 00:09:34 Does that mean we can just drop all references to the "R" flag and resetting flags, and just tell people that flags only apply to the variable they immediately precede? By: Anthony Minessale (anthm) 2005-05-04 09:33:24 If you set options (an arg with no '=') they will be effective on every subsequent arg unless it reaches another options string at which time it will reset the options to the new choice therefore R despite needing no code will clear all the flags for you. SetVar(abc|ex1=true|ex2=true|xyz|ex3=true|ex4=true) for ex1-ex2 the abc will be in effect once i set xyz it resets all the options to xyz (all described in the body of the patch and easily discerned from the 10 lines of code the function contains) I don't know how much more dialog we need here this is a huge bug that makes my feature look bad and makes people ask me for support despite the fact that it worked fine when I submitted the original patch, (happening a lot lately) This kinda stuff where 'fixes' are beaten into the ground makes me never want to fix stuff anymore, sigh I had to jump through hoops to even reply to this bug cos the stupid mantis keeps sending me to bugs2 where i am not logged in, forgot my pass, and to top it off the mantis says "reminder sent" and never sends it.  grrr Let me spare you the time to draft some reply putting me in my place and I'll just pretend I already read it so I can go do other stuff.. By: Kevin P. Fleming (kpfleming) 2005-05-04 11:53:38 I have committed changes to CVS HEAD to solve the underlying issues here, after much discussion with anthm. Basically, the SetVar() options syntax has been broken since it was committed, because of a change that was made during the commit. I have corrected that, so the 'g' flag now operates as documented. I have also removed the 'c' and 'r' flags, as their functionality can now be provided using the CDR() function instead (after having added 'r' support to the CDR() function). Finally, there was leftover CDR() support in pbx_builtin_setvar_helper that was not ever being invoked due to the presence of the CDR() function; it has been removed. If there is a remaining desire for SetVar to support multiple options strings, that issue can be addressed in another bug, with a new patch. By: Digium Subversion (svnbot) 2008-01-15 15:33:15.000-0600 Repository: asterisk Revision: 5574 U   trunk/include/asterisk/cdr.h U   trunk/pbx.c ------------------------------------------------------------------------ r5574 | kpfleming | 2008-01-15 15:33:15 -0600 (Tue, 15 Jan 2008) | 6 lines remove hardcoded CDR() support from pbx_builtin_setvar_helper in favor of already-implemented CDR() function make SetVar() options actually work as documented remove SetVar() 'c' and 'r' options, since the CDR() function can provide this functionality add 'r' option to CDR() function to control recursive retrieval/storage (inspired by bug ASTERISK-4045, but without the SetVar syntax changes) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=5574