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

[rmudgett]

> On March 21, 2012, 2:43 p.m., Mark Michelson wrote:
> > 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!

_t_ versions created


> On March 21, 2012, 2:43 p.m., Mark Michelson wrote:
> > /trunk/main/astobj2.c, line 654
> > <https://reviewboard.asterisk.org/r/1824/diff/1/?file=26492#file26492line654>
> >
> >     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.

Done


> On March 21, 2012, 2:43 p.m., Mark Michelson wrote:
> > /trunk/tests/test_astobj2.c, lines 614-620
> > <https://reviewboard.asterisk.org/r/1824/diff/1/?file=26493#file26493line614>
> >
> >     You should add some sort of check here to be sure that obj2 contains the data you expect it to have.

Done


> On March 21, 2012, 2:43 p.m., Mark Michelson wrote:
> > /trunk/tests/test_astobj2.c, lines 632-634
> > <https://reviewboard.asterisk.org/r/1824/diff/1/?file=26493#file26493line632>
> >
> >     If obj2 is non-NULL here, the test should fail.

Done


> On March 21, 2012, 2:43 p.m., Mark Michelson wrote:
> > /trunk/tests/test_astobj2.c, lines 602-604
> > <https://reviewboard.asterisk.org/r/1824/diff/1/?file=26493#file26493line602>
> >
> >     If obj2 is non-NULL then this test should fail.

Done


- rmudgett


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


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/cdfdc4ce/attachment.htm>