[asterisk-dev] murf: branch 1.4 r139347 - in /branches/1.4: main/pbx.c res/res_features.c

[Russell Bryant]

SVN commits to the Digium repositories wrote:
> Author: murf
> Date: Thu Aug 21 18:03:50 2008
> New Revision: 139347
> 
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=139347
> Log:
> 
> (closes issue #13251)
> Reported by: sergee
> Tested by: murf
> 
> THis is a bold move for a static release fix, but I wouldn't have
> made it if I didn't feel confident (at least a *bit* confident)
> that it wouldn't mess everyone up.
> 
> The reasoning goes something like this:
> 
> 1. We simply cannot do anything with CDR's at the current point
> (in pbx.c, after the __ast_pbx_run loop). It's way too late to
> have any affect on the CDRs. The CDR is already posted and gone,
> and the remnants have been cleared.
> 
> 2. I was very much afraid that moving the running of the 'h'
> extension down into the bridge code (where it would be now
> practical to do it), would result in a lot more calls to the
> 'h' exten, so I implemented it as another exten under another
> name, but found, to my pleasant surprise, that there was a 
> 1:1 correspondence to the running of the 'h' exten in the
> pbx_run loop, and the new spot at the end of the bridge.
> So, I ifdef'd out the current 'h' loop, and moved it into
> the bridge code. The only difference I can see is the stuff
> about the AST_PBX_KEEPALIVE, and hopefully, if this 
> is still an important decision point, I can replicate it
> if there are complaints. To be perfectly honest,
> the KEEPALIVE situation is not totally clear to me,
> and how it relates to a post-bridge situation is less
> clear. I suspect the users will point out everything
> in total clarity if this steps on anyone's toes!
> 
> 3. I temporarily swap the bridge_cdr into the channel
> before running the 'h' exten, which makes it possible
> for users to edit the cdr before it goes out the door.
> And, of course, with the endbeforehexten config var set,
> the users can also get at the billsec/duration vals.
> After the h exten finishes, the cdr is swapped back
> and processing continues as normal.
> 
> Please, all who deal with CDR's, please test this version
> of Asterisk, and file bug reports as appropriate!


As we discussed on IRC before you committed this code, this change is 
absolutely unacceptable for _any_ version of Asterisk.  It completely 
changes the meaning of the 'h' extension, which has been a core piece of 
Asterisk functionality for many years.

The obvious problems are:

1) What about calls that never involve the bridge code?  The 'h' 
extension won't even get executed?

2) As you even alluded to in your commit message, this introduces the 
possibility of running the 'h' extension multiple times for the same 
call, which has _never_ been the case before.


The issues raised by the bug report need to be addressed a different way.

-- 
Russell Bryant
Senior Software Engineer
Open Source Team Lead
Digium, Inc.