[asterisk-dev] [Code Review] Fix ao2_iterator API to hold references to containers being iterated.

[Russell Bryant]

> On 2009-09-30 10:25:18, Russell Bryant wrote:
> > I'm happy with this as is, without an API change.

To be more clear, after re reading the description, if there was going to be an additional change, I would be in favor of just marking ao2_iterator_init() as deprecated but leaving it there.


- Russell


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


On 2009-09-29 16:56:10, Kevin Fleming wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/383/
> -----------------------------------------------------------
> 
> (Updated 2009-09-29 16:56:10)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> See Mantis issue for details of what prompted this change.
> 
> Additional notes:
> 
> This patch changes the ao2_iterator API in two ways: F_AO2I_DONTLOCK has become an enum instead of a macro, with a name that fits our naming policy; also, it is now necessary to call ao2_iterator_destroy() on any iterator that has been created. Currently this only releases the reference to the container being iterated, but in the future this could also release other resources used by the iterator, if the iterator implementation changes to use additional resources.
> 
> The patch currently changes the API in a way that will *not* cause existing code to fail at compilation or run time; instead, existing code that is used against the new API implementation will result in a reference leak on the container being iterated. There are least two ways this can be addressed... the ao2_iterator_init() function's name can be changed so that existing code will fail to compile or link, or the existing function can be left alone (*without* taking a container reference) and a new function created that implements the API properly. If the latter option is chosen, ao2_iterator_init() would be marked 'deprecated' to encourage developers to switch to the new API function as soon as possible.
> 
> 
> This addresses bug 15987.
>     https://issues.asterisk.org/view.php?id=15987
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_queue.c 220997 
>   /trunk/channels/chan_console.c 220997 
>   /trunk/channels/chan_iax2.c 220997 
>   /trunk/channels/chan_sip.c 220997 
>   /trunk/funcs/func_dialgroup.c 220997 
>   /trunk/include/asterisk/astobj2.h 220997 
>   /trunk/main/astobj2.c 220997 
>   /trunk/res/res_calendar.c 220997 
>   /trunk/res/res_clialiases.c 220997 
>   /trunk/res/res_musiconhold.c 220997 
>   /trunk/res/res_odbc.c 220997 
>   /trunk/res/res_phoneprov.c 220997 
> 
> Diff: https://reviewboard.asterisk.org/r/383/diff
> 
> 
> Testing
> -------
> 
> Compile testing only at this point.
> 
> 
> Thanks,
> 
> Kevin
> 
>