[asterisk-dev] [Code Review]: Changes from Mantis ID 13495 in trunk.

[KNK]

> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/chan_dahdi.c, lines 979-988
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23472#file23472line979>
> >
> >     Update the Doxygen comments since these are no longer boolean.

Fixed in the next version of the patch


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/chan_dahdi.c, line 13976
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23472#file23472line13976>
> >
> >     CODING GUIDELINES: Always use curly braces.  Please go through your patch and add them where you add or modify existing lines.
> >     
> >     You don't have to fix formatting in lines you don't need to change.

I thought i got all of them. Will make another pass.


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.h, line 195
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23473#file23473line195>
> >
> >     This is never initialized and is likely no longer needed with recent changes to sig_ss7.

Removed in the next version of the patch


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.h, lines 197-200
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23473#file23473line197>
> >
> >     Update the Doxygen comments to reflect that it is no longer a boolean.

Fixed in the next version of the patch


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.h, line 247
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23473#file23473line247>
> >
> >     This could be made a boolean bit field.

Fixed in the next version of the patch. Moved to channel status bits and added Doxygen comment


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, line 645
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23474#file23474line645>
> >
> >     Curly brace position.

Fixed in the next version of the patch.


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, line 713
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23474#file23474line713>
> >
> >     Why was linkset->ss7 changed to p->ss7->ss7?  They should be the same.

Probably copy/paste result - restored.


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, line 811
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23474#file23474line811>
> >
> >     The following three switch statements are inconsistently formatted.  CODING GUIDELINES

Fixed in the next version of the patch.


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, lines 909-911
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23474#file23474line909>
> >
> >     This is always true since it is an array.

Fixed in the next version of the patch.


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, line 2028
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23474#file23474line2028>
> >
> >     This should be static.

Fixed in the next version of the patch.


> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.h, line 202
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23473#file23473line202>
> >
> >     Get rid of this flag.  Use call_level instead.  The call_level value represents the call state and eliminated the proceeding and alerting flags.

Fixed in the next version of the patch


- KNK


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


On Feb. 14, 2012, 4:54 a.m., KNK wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1676/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2012, 4:54 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> chan_dahdi / sig_ss7 part of changes
> 
> 
> This addresses bug SS7-27.
>     https://issues.asterisk.org/jira/browse/SS7-27
> 
> 
> Diffs
> -----
> 
>   trunk/channels/chan_dahdi.c 351501 
>   trunk/channels/sig_ss7.h 351501 
>   trunk/channels/sig_ss7.c 351501 
> 
> Diff: https://reviewboard.asterisk.org/r/1676/diff
> 
> 
> Testing
> -------
> 
> compiles, link setup, cli commands, bassic calls
> Passed my phone calls through local SS7 loop for few days without problems
> 
> 
> Thanks,
> 
> KNK
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120309/1f3fe67c/attachment-0001.htm>