[asterisk-dev] Re: [svn-commits] murf: branch 1.4 r67073 - /branches/1.4/main/cdr.c

[Steve Murphy]

On Tue, 2007-06-05 at 01:58 -0400, Leif Madsen wrote:
> On Monday 04 June 2007 17:59:35 svn-commits at lists.digium.com wrote:
> > Author: murf
> > Date: Mon Jun  4 16:59:34 2007
> > New Revision: 67073
> >
> > URL: http://svn.digium.com/view/asterisk?view=rev&rev=67073
> > Log:
> > This typo has been here since 1.4 forked. It has been the source of
> > heartburn to many a dialplan/CDR programmer.
> >
> > Modified:
> >     branches/1.4/main/cdr.c
> >
> > Modified: branches/1.4/main/cdr.c
> > URL:
> > http://svn.digium.com/view/asterisk/branches/1.4/main/cdr.c?view=diff&rev=6
> >7073&r1=67072&r2=67073
> > ===========================================================================
> >=== --- branches/1.4/main/cdr.c (original)
> > +++ branches/1.4/main/cdr.c Mon Jun  4 16:59:34 2007
> > @@ -749,7 +749,7 @@
> >  int ast_cdr_setcid(struct ast_cdr *cdr, struct ast_channel *c)
> >  {
> >  	for (; cdr; cdr = cdr->next) {
> > -		if (ast_test_flag(cdr, AST_CDR_FLAG_LOCKED))
> > +		if (!ast_test_flag(cdr, AST_CDR_FLAG_LOCKED))
> >  			set_one_cid(cdr, c);
> >  	}
> >  	return 0;
> >
> 
> What is this actually fixing? Would you mind elaborating for us unknowing?
> 
> Leif.

Sure!

If you look at all the other routines that do something to a CDR, you
will see the same basic structure; they all loop thru the cdr and any
CDR's attached to that CDR. If they are not locked, they apply the
requested change.

In this case, the test was inverted. It would only apply the change to
locked CDR's and leave unlocked CDR's alone.

The 1.2 version of this function didn't have this mistake. I think the
error crept in when set_one_cid() was added.

Why are LOCKED CDR's around? Because of ForkCDR. I've found some other
cases, but ForkCDR is the main way LOCKED cdrs are created. ForkCDR()
will dup a CDR, lock the original, and put the new CDR at the end of the
cdr list (among a few other things). Locked CDRs are meant not to be
changed.

The end result is that, in most cases, the CID info in the CDR's (src
and clid) never get updated at all. Even when you WANT them to be.

murf

-- 
Steve Murphy
Software Developer
Digium
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3227 bytes
Desc: not available
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20070605/b2db0e3a/smime.bin