[asterisk-dev] [Code Review] 3087: Mini-audit of the ao2_iterator loops in the new code files.

Matt Jordan reviewboard at asterisk.org
Fri Dec 20 13:32:40 CST 2013


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

Ship it!



/branches/12/res/ari/resource_channels.c
<https://reviewboard.asterisk.org/r/3087/#comment19943>

    I had to look at this several times. This actually should explode pretty spectacularly in creative ways, since we are decrementing the snapshots more than once. Someone is going to be unhappy when the channel snapshot is nuked prior to its full expected lifetime.
    
    I personally dislike this use of RAII_VAR - it's wasteful in its processing and it takes careful looking to understand that its cleaning up after the bump from ao2_iterator_next; but at least we're not mixing it with the for loop now.


- Matt Jordan


On Dec. 20, 2013, 1:26 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3087/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2013, 1:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> * Fixed several places where ao2_iterator_destroy() was not called.
> 
> * Fixed several iterator loop object variable reference problems.
> 
> * Fixed res_parking AMI actions returning non-zero.  Only the AMI logoff action can return non-zero.
> 
> 
> Diffs
> -----
> 
>   /branches/12/tests/test_stasis.c 404420 
>   /branches/12/tests/test_scoped_lock.c 404420 
>   /branches/12/tests/test_cel.c 404420 
>   /branches/12/res/res_pjsip/location.c 404420 
>   /branches/12/res/parking/parking_manager.c 404420 
>   /branches/12/res/ari/resource_endpoints.c 404420 
>   /branches/12/res/ari/resource_channels.c 404420 
>   /branches/12/res/ari/resource_bridges.c 404420 
> 
> Diff: https://reviewboard.asterisk.org/r/3087/diff/
> 
> 
> Testing
> -------
> 
> Compiler only.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131220/0a0868da/attachment.html>


More information about the asterisk-dev mailing list