[asterisk-dev] [Code Review] An alternate way of accomplishing the goal of review 295

[Kevin Fleming]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/298/#review903
-----------------------------------------------------------



/branches/1.4/channels/chan_sip.c
<http://reviewboard.digium.com/r/298/#comment2168>

    To reduce code duplication, I'd suggest storing the result of __sip_semi_ack/__sip_ack in a variable, and then testing and making the ignore decision.



/branches/1.4/channels/chan_sip.c
<http://reviewboard.digium.com/r/298/#comment2169>

    Where did these flag sets move to? Are they no longer necessary?


- Kevin


On 2009-06-26 14:45:53, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/298/
> -----------------------------------------------------------
> 
> (Updated 2009-06-26 14:45:53)
> 
> 
> Review request for Asterisk Developers and Kevin Fleming.
> 
> 
> Summary
> -------
> 
> In review 295 (https://reviewboard.asterisk.org/r/295), I proposed a method of solving issue 11231. Even though I already have received a "Ship it!" from a couple of people, Kevin told me about an alternate way of approaching the problem, so I coded up a new way of handling this.
> 
> Since we have to traverse the list of packets on the sip_pvt, remove the retransmission of the packet from the scheduler, and free the packet anyway, we can use that function to determine if the received response is one we are expecting. This required modifying the __sip_ack function to return a boolean instead of returning void.
> 
> This approach requires no code additions, but requires rearrangement of existing code. While making this change, I noticed that the "ignore" parameter in handle_response could never be TRUE and that it wasn't even used anyway, so I removed it. This change also made the lastnoninvite field on a sip_pvt useless, so I removed it as well.
> 
> Let me know if you think this is a better approach than what was proposed in review 295. The only thing that worries me is that this may be a bit harder to merge to other branches than the 295 method. Thanks!
> 
> 
> This addresses bug 11231.
>     https://issues.asterisk.org/view.php?id=11231
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/channels/chan_sip.c 202966 
> 
> Diff: http://reviewboard.digium.com/r/298/diff
> 
> 
> Testing
> -------
> 
> I performed the same test as per review 295.
> 
> 
> Thanks,
> 
> Mark
> 
>