[asterisk-dev] [Code Review] Call Completion: libpri component

[rmudgett at digium.com]

> On 2010-02-25 18:00:11, David Vossel wrote:
> > /branches/1.4/pri_cc.c, lines 660-667
> > <https://reviewboard.asterisk.org/r/522/diff/1/?file=8187#file8187line660>
> >
> >     you could just do
> >     
> >     struct rose_msg_invoke msg = { 0, };
> >     
> >     and I think that initializes everything without having to do memset.  The only reason I'm pointing this out is that it makes the job of the reviewer much easier.  When I see a struct declared on the stack the first thing I look for is to see where it is initialized.  If it is just right there I don't have to hunt around for it.

Two things:
1) Initializing at declaration would be wasted by the early return.
2) Initializing at the point of initial use makes as much sense.  The memset() in this case is part of filling in the structure with data to send in the facility message.


> On 2010-02-25 18:00:11, David Vossel wrote:
> > /branches/1.4/pri_cc.c, lines 610-612
> > <https://reviewboard.asterisk.org/r/522/diff/1/?file=8187#file8187line610>
> >
> >     maybe this was intended. it just looked odd to me.  I saw this quite a bit.

The indent utility likes to do this when the initial line is longer than the set length so I just left it.  It takes work to undo it.  If you run indent on the file again, it undoes your work.


- rmudgett


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/522/#review1601
-----------------------------------------------------------


On 2010-02-19 16:51:38, rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/522/
> -----------------------------------------------------------
> 
> (Updated 2010-02-19 16:51:38)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Call Completion Supplementary Service (CCSS) added for the following switch types: ETSI PTMP, ETSI PTP, Q.SIG.
> Specifications:
> ETS 300 359 CCBS for PTMP and PTP
> ETS 301 065 CCNR for PTMP and PTP
> ECMA-186 Call Completion for Q.SIG
> 
> Several support services were added to support CC:
> Dummy Call Reference.
> Q.931 REGISTER message.
> Dynamic expansion of the number of available timers (up to 8192).
> Enhanced facility message handling.
> 
> Current implementation limitations preclude the following:
> CC service retention is not supported.
> Q.SIG path reservation is not supported.
> 
> The review for the asterisk component is located: https://reviewboard.asterisk.org/r/523/
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/Makefile 1499 
>   /branches/1.4/doc/cc_ptmp_agent.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_ptmp_agent_flattened.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_ptmp_monitor.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_ptmp_monitor_flattened.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_ptp_agent.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_ptp_agent_flattened.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_ptp_monitor.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_ptp_monitor_flattened.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_qsig_agent.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_qsig_agent_flattened.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_qsig_monitor.fsm PRE-CREATION 
>   /branches/1.4/doc/cc_qsig_monitor_flattened.fsm PRE-CREATION 
>   /branches/1.4/libpri.h 1499 
>   /branches/1.4/pri.c 1499 
>   /branches/1.4/pri_cc.c PRE-CREATION 
>   /branches/1.4/pri_facility.h 1499 
>   /branches/1.4/pri_facility.c 1499 
>   /branches/1.4/pri_internal.h 1499 
>   /branches/1.4/pri_q931.h 1499 
>   /branches/1.4/prisched.c 1499 
>   /branches/1.4/q931.c 1499 
>   /branches/1.4/rose.h 1499 
>   /branches/1.4/rose.c 1499 
>   /branches/1.4/rose_etsi_cc.c PRE-CREATION 
>   /branches/1.4/rose_internal.h 1499 
>   /branches/1.4/rose_qsig_cc.c PRE-CREATION 
>   /branches/1.4/rosetest.c 1499 
> 
> Diff: https://reviewboard.asterisk.org/r/522/diff
> 
> 
> Testing
> -------
> 
> A lot. :)
> 
> Unit tests were added to rosetest to test the encode/decode of the new ROSE call completion messages.
> 
> 
> Thanks,
> 
> rmudgett
> 
>