Summary:	ASTERISK-07124: CDR vars are read-only unneccesarily.
Reporter:	xrobau (xrobau)	Labels:
Date Opened:	2006-06-08 07:16:24	Date Closed:	2011-06-07 14:07:25
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	Quoting from cdr.c /* readonly cdr variables */ static const char *cdr_readonly_vars[] = { "clid", "src", "dst", "dcontext", "channel", "dstchannel","lastapp", "lastdata", "start", "answer", "end", "duration", "billsec", "disposition", "amaflags", "accountcode", "uniqueid", "userfield", NULL }; Some variables, obviously, should be read-only as it would be foolish to try to set them (eg, 'duration', which isn't known about until the call is finished), however, you may be overriding the CLID set on an external call, but you still want to know which device made the call. ****** ADDITIONAL INFORMATION ****** If you do: NoOp(${CALLERID(num)}) ; results in 300, your stations caller ID Set(CALLERID(num)=1234) Dial(IAX2/foo, 4567) The CDR record contains src and clid set to 1234. The only way to get the original caller ID is to hope that it's the same as the SIP/IAX channel the call is coming from and parse it from the 'channel' record. This is fine, and quite possibly expected, unless you actually care about the device that made the call. You would assume that this would work, to bypass this issue: Set(ORIGCALLID=${CALLERID(num)}) Set(CALLERID(num)=1234) Set(CDR(src)=${ORIGCALLID}) ; This results in cdr.c: Attempt to set a read-only variable!. Dial(IAX2/foo, 4567) However, 'clid', 'src', and 'dst' are, probably incorrectly, marked as read-only vars. I would suggest that those three, as well as accountcode and userfield, which are specifically there _to_ be set, should not be marked read only. I did try taking the 'clid', 'src' and 'dst' out of the readonly_vars list, but when ast_cdr_setcid is called, it doesn't check to see if cdr->clid has already been set to something, and happily overwrites it (I'm assuming this, I didn't actually check the flow of the code, but that code definately does _not_ check for anything in cdr->clid before overwriting it)
Comments:	By: Serge Vecher (serge-v) 2006-06-08 09:12:57 xrobau: I doubt this will be changed for 1.2, so moving this issue to trunk. In the feature, it is probably best to email the asterisk-dev list first to solicit feedback from the community. Or join the #asterisk-dev channel on IRC. By: xrobau (xrobau) 2006-06-08 09:27:22 I concur that it's probably not a 1.2 thing - however, it is specified in the 'show function CDR' as 'Gets or sets a CDR variable' - at the moment it doesn't set any CDR variables, and (I assume) you need to use the depreciated SetCDRUserField and SetAccount applications to set the bare minimum. Possibly a documentation fix in 1.2, and a proper fix in 1.4? By: Serge Vecher (serge-v) 2006-06-08 09:29:57 sounds good to me. I would still email the dev-list with a summary referencing this bug -- this way the issue gets maximum exposure. By: Joshua C. Colp (jcolp) 2006-06-09 12:27:24 This is a tricky thing to approach... because it will set CDR variables, that is - variables not part of a regular CDR record. For those you must use the applicable method if they can be changed. The documentation could be considered a bit misleading depending on how you read it. By: Tilghman Lesher (tilghman) 2006-06-12 10:38:19 Agree.  Although the source code is probably confusing, this is not really an issue.  The point of this code is to ensure that the bits of the CDR which have direct structure names don't get inserted in the attached variable list (mainly because they'll never get referenced).  While we could change the comment, the code is completely correct.