> On Dec. 12, 2013, 11:48 a.m., opticron wrote: > > /branches/12/funcs/func_cdr.c, lines 472-474 > > <https://reviewboard.asterisk.org/r/3057/diff/2/?file=49301#file49301line472> > > > > Return and reduce indentation. > > Matt Jordan wrote: > Normally I'd agree, but that approach has one drawback here. > > Providing block scope to the code path that requires a Stasis > subscription lets me RAII_VAR the subscription such that it is only > allocated/cleaned up if we are operating on a channel. The code path that > performs a direct read callback actually gets hit a lot - all of the adaptive > CDR backends use that approach to write out CDR records. Saving ourselves 16+ > allocations on each CDR write is a good thing. > > If I check the channel name length and return, I'd still want block scope > for the subscription - which means I don't actually reduce indentation here > at all.
The allocation is already broken out from the RAII_VAR and can stay with the code it is relevant to which would prevent undue allocations while still affording the benefits of RAII_VAR. The end result would be moving the RAII_VAR with NULL initializer up to the top of the function. - opticron ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3057/#review10394 ----------------------------------------------------------- On Dec. 12, 2013, 12:03 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3057/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2013, 12:03 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-22884 and ASTERISK-22886 > https://issues.asterisk.org/jira/browse/ASTERISK-22884 > https://issues.asterisk.org/jira/browse/ASTERISK-22886 > > > Repository: Asterisk > > > Description > ------- > > When doing the rework of the CDR engine that pushed all of the logic into > cdr.c and made it respond to changes in channel state over Stasis, I knew > that accessing the CDR engine from the dialplan would be "slightly" > non-deterministic. Dialplan threads would be accessing CDRs while Stasis > threads would be updating the state of said CDRs - whereas in the past, > everything happened on the dialplan threads. Tests have shown that "slightly" > is in reality "very". > > This patch synchronizes things by making the dialplan applications/functions > that manipulate CDRs do so over Stasis. ForkCDR, NoCDR, ResetCDR, CDR, and > CDR_PROP now all use Stasis to send their requests over to the CDR engine, > and synchronize on the Stasis subscription so that they return their > values/control to the dialplan at the appropriate time. > > While going through this, the following changes were also made: > * DISA, which can reset the CDR when a user successfully authenticates, now > just uses the ResetCDR app to do this. This prevents having to duplicate the > same Stasis synchronization logic in that application. > * Answer no longer disables CDRs. It actually didn't work anyway - calling > DISABLE on the channel's CDR doesn't stop the CDR from getting the Answer > time - it just kills all CDRs on that channel, which isn't what the caller > would intend. > > > Diffs > ----- > > /branches/12/main/pbx.c 403703 > /branches/12/main/cdr.c 403703 > /branches/12/include/asterisk/cdr.h 403703 > /branches/12/funcs/func_cdr.c 403703 > /branches/12/apps/app_forkcdr.c 403703 > /branches/12/apps/app_disa.c 403703 > /branches/12/apps/app_cdr.c 403703 > /branches/12/UPGRADE.txt 403703 > /branches/12/CHANGES 403703 > > Diff: https://reviewboard.asterisk.org/r/3057/diff/ > > > Testing > ------- > > * The CDR tests pass (now deterministicly) > * The hangup handler tests now pass, as they can get reliable values in the h > extension from the CDR engine > > > Thanks, > > Matt Jordan > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
