[asterisk-dev] [Code Review] Add global ao2 array container.

[Mark Michelson]

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


In addition to the macros you have defined in astobj2.h, you should also define _t_ versions of them to aid when debugging refcount issues.

You really should find some place in the code to use this, even if it's for something basic. It's frowned upon to add APIs and then not actually use them anywhere.

Also, big props for writing a test case!


/trunk/main/astobj2.c
<https://reviewboard.asterisk.org/r/1824/#comment10686>

    I think that here and everywhere else you use ao2_ref or __ao2_ref(), you should replace it with __ao2_ref_debug(). This way, you can pass along the file, line, and function that will in 99% of cases be more helpful for someone trying to debug refcount issues. At least do this when REF_DEBUG is defined.



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/1824/#comment10683>

    If obj2 is non-NULL then this test should fail.



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/1824/#comment10684>

    You should add some sort of check here to be sure that obj2 contains the data you expect it to have.



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/1824/#comment10685>

    If obj2 is non-NULL here, the test should fail.


- Mark


On March 21, 2012, 12:23 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1824/
> -----------------------------------------------------------
> 
> (Updated March 21, 2012, 12:23 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Summary
> -------
> 
> Global ao2 objects must always exist after initialization because there is no access control to obtain another reference to the global object.
> 
> It is anticipated that the Asterisk 11 Configuration Management project could use these new API calls to replace an active configuration parameter object with an updated configuration parameter object.
> 
> With these new API calls, the global object could be replaced, removed, or referenced without the risk of someone using a stale global object pointer.
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/astobj2.h 360188 
>   /trunk/main/astobj2.c 360188 
>   /trunk/tests/test_astobj2.c 360188 
> 
> Diff: https://reviewboard.asterisk.org/r/1824/diff
> 
> 
> Testing
> -------
> 
> Added a new unit test to exercise the new API calls.  They pass.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120321/0c442056/attachment.htm>