[asterisk-dev] [Code Review] chan_sip refcount cleanup derived from twilsons's review 1207

Terry Wilson reviewboard at asterisk.org
Wed May 11 13:55:40 CDT 2011



> On 2011-05-09 16:47:49, jrose wrote:
> > /tags/1.8.3.3/channels/chan_sip.c, lines 3887-3905
> > <https://reviewboard.asterisk.org/r/1210/diff/1/?file=16416#file16416line3887>
> >
> >     Since this function is just returning 0 every time regardless of other circumstances, couldn't it just be a void function instead?
> 
> Rob Gagnon wrote:
>     In the long term, probably yes, but some areas of the code branch based on the return value of the function.   At this time, the feeling is not to alter the code more than needed.  This patch is designed to mirror the approved 1.6.2 patch under review 1207, without any feature-creep.

I also added the fix unreffing sip_monitor_instances in unload_module which I think your original patch had (that 1.6.2 didn't need).


- Terry


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


On 2011-05-09 16:40:49, Rob Gagnon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1210/
> -----------------------------------------------------------
> 
> (Updated 2011-05-09 16:40:49)
> 
> 
> Review request for Asterisk Developers, Terry Wilson, David Vossel, and Rob Gagnon.
> 
> 
> Summary
> -------
> 
> This patch is the altered version of Terry's patch for chan_sip from review 1207 (which was for branch 1.6.2)
> 
> This patch is for tag 1.8.3.3, which should apply pretty clean to trunk and to branch 1.8
> 
> 5 alterations were done to fit the 1.8 version of chan_sip.c.  The other 21 hunks applied either with simple offsets, or fuzz 1/2.
> 
> 1) a. remove peer_is_marked() function
>    b. move unlink_marked_peers_from_tables()
>    c. move unlink_peer_from_tables()
> 
> 2) manual patch of the code in sip_cancel_destroy() 
> 
> 3) find_call() did not need <<< p = dialog_unref(p, "pedantic linear search for dialog unref to restart search"); >>> due to the new iterator in place of the original goto loop.
> 
> 4) changed the comment from "/* returns p locked and ref'd */" to "/* returns p ref'd */"
> 
> 5) manunal patch of the code in start_session_timer()
> 
> 
> This addresses bug 17255.
>     https://issues.asterisk.org/view.php?id=17255
> 
> 
> Diffs
> -----
> 
>   /tags/1.8.3.3/channels/chan_sip.c 318283 
> 
> Diff: https://reviewboard.asterisk.org/r/1210/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rob
> 
>

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


More information about the asterisk-dev mailing list