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

[KNK]

> On Oct. 1, 2012, 4:20 p.m., rmudgett wrote:
> > Please create a new libss7 review for the libss7 patch.  I cannot comment on it.  The ISUP_SET_OUT flag name is not descriptive of what it does.
> > 
> > This review is also getting sluggish because of its size.  You should be able to create a new review against the ss7_27_knk branch.  Reviewboard is fussy about the branch names.  This might work when you create the new review and need to specify the branch name: /team/rmudgett/ss7_27_knk/

OK. Should i create a new review or use this one and simply change the base branch?

Will change ISUP_SET_OUT to ISUP_PENDING_IAM.


> On Oct. 1, 2012, 4:20 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, lines 768-772
> > <https://reviewboard.asterisk.org/r/1676/diff/15/?file=31400#file31400line768>
> >
> >     The isup_init_call() should be merged into the isup_new_call().  Then the function signature would be:
> >     struct isup_call *isup_new_call(struct ss7 *ss7, int cic, unsigned int dpc);
> >     
> >     The adding of the new parameter to isup_init_call() is not a good idea.  It would be better to add a new call to indicate that there is a pending IAM for the isup_call.
> >     void isup_call_pending(struct isup_call *call);

Great idea - isup_new_call() now calls isup_init_call() and sets the flag (in libss7)


> On Oct. 1, 2012, 4:20 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, line 2697
> > <https://reviewboard.asterisk.org/r/1676/diff/15/?file=31400#file31400line2697>
> >
> >     This line does not belong here.  Simply moving lines blindly does not work.  The lines must be adapted to their new location in the code.
> >     
> >     At this point, the p->do_hangup should be set to SS7_HANGUP_DO_NOTHING since if ast_hangup() is called on the channel before the IAM is sent, there is noting to do.

Oops, fixed.


> On Oct. 1, 2012, 4:20 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, lines 2935-2938
> > <https://reviewboard.asterisk.org/r/1676/diff/15/?file=31400#file31400line2935>
> >
> >     Need to also check if the soft-hangup flags were set here to abort the outgoing call.  sig_ss7_cb_hangup() sets the AST_SOFTHANGUP_DEV flag on a collision.
> >     
> >     Right before the IAM is sent near the end of sig_ss7_call(), the p->do_hangup should be set to SS7_HANGUP_SEND_REL.

Done


- KNK


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


On Sept. 25, 2012, 9:52 a.m., KNK wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1676/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2012, 9:52 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/sig_ss7.c 373581 
>   trunk/channels/sig_ss7.h 373581 
>   trunk/channels/chan_dahdi.c 373581 
>   trunk/configs/chan_dahdi.conf.sample 373581 
>   trunk/configs/ss7.timers.sample PRE-CREATION 
>   trunk/configure.ac 373581 
> 
> Diff: https://reviewboard.asterisk.org/r/1676/diff
> 
> 
> Testing
> -------
> 
> compiles, link setup, cli commands, bassic calls, connected line and redirection.
> 
> 
> Thanks,
> 
> KNK
> 
>

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