[asterisk-dev] [Code Review] Implement ast_channel_search_locked to expensive per-notify channel walk in chan_sip

[Russell Bryant]

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



/trunk/channels/chan_sip.c
<http://reviewboard.digium.com/r/28/#comment120>

    Go ahead and set the caller pointer to NULL at the end of this block to be explicit that it can't be touched after it gets unlocked.



/trunk/include/asterisk/channel.h
<http://reviewboard.digium.com/r/28/#comment119>

    Document that the filter is called with the channel locked (after making my requested change to do so).
    
    Also, indicate that the channel that is returned gets returned locked.



/trunk/main/channel.c
<http://reviewboard.digium.com/r/28/#comment118>

    I would lock the channel before calling the filter.  In your filter implementation, the channel needs to be locked there.  Putting it outside the filter call will prevent the unlock and immediately lock again in the case that we found the match, and also simplify the filter code.


- Russell


On 2008-11-04 16:27:50, Sean Bright wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/28/
> -----------------------------------------------------------
> 
> (Updated 2008-11-04 16:27:50)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Based on some feedback from my chan_sip commit earlier today from oej, kpfleming, and russellb, implement ast_channel_search_locked which iterates over the channel list, while locked, and calls a filter callback to determine if this is the channel we are looking for.  If so, we lock and return, otherwise we return NULL.
> 
> Patch also includes the change required to chan_sip.c to use the new function.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 154189 
>   /trunk/include/asterisk/channel.h 154189 
>   /trunk/main/channel.c 154189 
> 
> Diff: http://reviewboard.digium.com/r/28/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sean
> 
>