[asterisk-dev] [Code Review] astobj2 unit test

[David Vossel]

> On 2010-02-09 18:27:29, Russell Bryant wrote:
> > /trunk/tests/test_astobj2.c, lines 248-252
> > <https://reviewboard.asterisk.org/r/496/diff/1/?file=8066#file8066line248>
> >
> >     Another interesting test you could do at this point is an ao2_callback() on the whole container to make sure the ref count on every object is 1.  ao2_ref(obj, 0) gives you the current ref count.

I could add that check, but I don't know if it is necessary (plus it's another callback to every item in the container, one of the tests is 100,000 items).  If the ref count isn't perfect at this point we'll know about it when the destructor_count is checked at the end of the function.


> On 2010-02-09 18:27:29, Russell Bryant wrote:
> > /trunk/tests/test_astobj2.c, line 285
> > <https://reviewboard.asterisk.org/r/496/diff/1/?file=8066#file8066line285>
> >
> >     It could be that the destructor was called too many times, as well

true, so in that case I suppose destructor_count would be negative.


> On 2010-02-09 18:27:29, Russell Bryant wrote:
> > /trunk/tests/test_astobj2.c, lines 310-315
> > <https://reviewboard.asterisk.org/r/496/diff/1/?file=8066#file8066line310>
> >
> >     Does the lim variable provide any benefit?  Perhaps you were going to include in some status messages?

That was left over from previous logic that went away.  The lim variable should go away now as well.


> On 2010-02-09 18:27:29, Russell Bryant wrote:
> > /trunk/tests/test_astobj2.c, lines 218-223
> > <https://reviewboard.asterisk.org/r/496/diff/1/?file=8066#file8066line218>
> >
> >     This looks wrong.  I don't understand the "remove ref from link" line.

I struggled with this and I'm glad you said something. I had convinced myself I was wrong but I think it makes sense now.  There is a bug in astobj2.c when OBJ_MULTIPLE | OBJ_UNLINK is used.  Usually when OBJ_UNLINK is used the ref from the container is kept but the object is removed from the container.  When OBJ_MULTIPLE is used with OBJ_UNLINK each item is linked to a newly created container that lives within the iterator that is returned.  The problem with this is that the ref kept by the old container still remains.  Essentially we lose track of a ref.  I'll fix this with my next update.  This is very confusing code.


> On 2010-02-09 18:27:29, Russell Bryant wrote:
> > /trunk/tests/test_astobj2.c, lines 233-235
> > <https://reviewboard.asterisk.org/r/496/diff/1/?file=8066#file8066line233>
> >
> >     What does this loop test?  Just make sure it doesn't crash?

This was because I noticed odd behavior by the iterator returned by OBJ_MULTIPLE.  The OBJ_MULTIPLE code is complex so I added some tests like this to verify we keep up with the ref count correctly in all possible scenarios.


- David


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


On 2010-02-09 14:44:54, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/496/
> -----------------------------------------------------------
> 
> (Updated 2010-02-09 14:44:54)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is a unit test for main/astobj2.c.  All the status update calls will be changed to match Russell's tweak once it is committed.  That is why I do not set the error string anywhere.
> 
> 
> Diffs
> -----
> 
>   /trunk/tests/test_astobj2.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/496/diff
> 
> 
> Testing
> -------
> 
> ran unit test, verified it passes. 
> 
> 
> Thanks,
> 
> David
> 
>