[asterisk-dev] [Code Review] Extract sig_ss7 out of chan_dahdi.

[rmudgett at digium.com]

> On 2010-05-26 14:28:30, pabelanger wrote:
> > Another set of eyes for formatting.  95% of comments are about missing braces. ;)
> > 
> > Good work.

The intention was to extract the code from chan_dahdi not to reformat it.  Any reformatting would make merging from earlier versions difficult.  Not reformatting certainly made my life easier when merging changes for chan_dahdi.c/sig_pri.c.

Thanks for the review.

The formatting in chan_dahdi is terrible. :)  It was written before the coding guidelines were stressed.  I have been making incremental improvements to the formatting as I need to change the code.


> On 2010-05-26 14:28:30, pabelanger wrote:
> > /trunk/channels/sig_ss7.c, line 782
> > <https://reviewboard.asterisk.org/r/661/diff/3/?file=10212#file10212line782>
> >
> >     Is this considered a mid-block variables?

No.  (If I found the correct variable definition.)  This variable is declared at the beginning of the if block.  A mid-block declaration would be declared in between code statements.
For example:
if (test) {
  foo();
  int var = 3;
  bar(var);
}


> On 2010-05-26 14:28:30, pabelanger wrote:
> > /trunk/channels/sig_ss7.c, lines 131-132
> > <https://reviewboard.asterisk.org/r/661/diff/3/?file=10212#file10212line131>
> >
> >     To be removed?

The intent of these comments is to show what the value should be and already is because of the ast_party_caller_init() call.  The comments also show that the structure member elements were not forgotten about.


- rmudgett


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


On 2010-05-14 17:31:07, rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/661/
> -----------------------------------------------------------
> 
> (Updated 2010-05-14 17:31:07)
> 
> 
> Review request for Asterisk Developers and mattf.
> 
> 
> Summary
> -------
> 
> Extract the SS7 specific code out of chan_dahdi like what was done to ISDN/PRI and analog signaling.  The new SS7 structures were modeled on sig_pri.
> 
> The changes to sig_pri are an enhancement and a bug fix made possible because SS7 was extracted.
> 1) The sig_pri TRANSFERCAPABILITY channel variable should have been set unconditionally in sig_pri_new_ast_channel().
> 
> 2) SS7/PRI transfer capability interaction in dahdi_new() fixed because of SS7 extraction.
> 
> 3) Module ref count error in dahdi_new() if startpbx failed to start the PBX for some reason.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/Makefile 263201 
>   /trunk/channels/chan_dahdi.c 263201 
>   /trunk/channels/sig_pri.h 263201 
>   /trunk/channels/sig_pri.c 263201 
>   /trunk/channels/sig_ss7.h PRE-CREATION 
>   /trunk/channels/sig_ss7.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/661/diff
> 
> 
> Testing
> -------
> 
> Made test calls between T1 ports on a test box.
> 
> 
> Thanks,
> 
> rmudgett
> 
>