[asterisk-dev] [Code Review] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.

George Joseph reviewboard at asterisk.org
Thu Jun 19 16:25:55 CDT 2014



> On June 19, 2014, 1:31 p.m., rmudgett wrote:
> > branches/12/main/astobj2_rbtree.c, lines 1324-1330
> > <https://reviewboard.asterisk.org/r/3593/diff/3/?file=59520#file59520line1324>
> >
> >     This is common code to any astobj2 container and should be pulled back up to the general container code in the AO2_CONTAINER_INSERT_NODE_INSERTED case.
> >     
> >     The goto inserted changes in this function could be reverted as a result.
> >     
> >     The hash container specific hash_ao2_link_node_stat() call that is done in the corresponding location of hash_ao2_insert_node() would still be done there.

I pushed some of this stuff down for 2 reasons...
1.  I think it's the container implementation's responsibility to make sure it's own stats are updated and the container is consistent, even though some of the code might be duplicated among container implementations.
2.  It makes the common code harder to read and follow.

Having said that...  :)
I pulled them back up.


- George


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


On June 10, 2014, 8:42 a.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3593/
> -----------------------------------------------------------
> 
> (Updated June 10, 2014, 8:42 a.m.)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c.  This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details.  The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again.  If not, I'll remove it then.
> 
> 
> Diffs
> -----
> 
>   branches/12/tests/test_astobj2.c 415657 
>   branches/12/main/astobj2_rbtree.c 415657 
>   branches/12/main/astobj2_private.h 415657 
>   branches/12/main/astobj2_hash.c 415657 
>   branches/12/main/astobj2_container_private.h 415657 
>   branches/12/main/astobj2_container.c 415657 
>   branches/12/build_tools/cflags.xml 415657 
> 
> Diff: https://reviewboard.asterisk.org/r/3593/diff/
> 
> 
> Testing
> -------
> 
> Ran all the test framework tests plus the "astobj2 test" test.
> Ran testsuite tests/channels/pjsip.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

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


More information about the asterisk-dev mailing list