[asterisk-dev] [Code Review] Re-work MoH class object handling

[Russell Bryant]

> On 2008-12-19 16:15:34, Jeff Peeler wrote:
> > /branches/1.4/res/res_musiconhold.c, line 779
> > <http://reviewboard.digium.com/r/106/diff/1/?file=2065#file2065line779>
> >
> >     The inuse variable here was used to fix issue 13229, which seems to be present again in your branch.

Good catch.

After discussion on IRC, we decided to remove this if check completely, since the core issue in issue 13229 for this change was that pid could be 0 in valid circumstances.  Also, inuse being 0 should never be possible.


- Russell


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/106/#review272
-----------------------------------------------------------


On 2008-12-19 15:23:51, Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/106/
> -----------------------------------------------------------
> 
> (Updated 2008-12-19 15:23:51)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is a big change to res_musiconhold which re-works how object management was being done for the MoH classes.
> 
> Previously, there was a hand-rolled use count being used.  However, there have been some crashes reported in this module, and I am pretty confident that there are errors in how the reference counts were being handled before.  So, I have reworked this to use astobj2 and made sure that references were handled in all places that they should be.
> 
> There should be no noticeable changes from the user point of view, other than (hopefully) increased stability.  From a developer point of view, this makes the code more streamlined with how we handle reference counts elsewhere in Asterisk.
> 
> Finally, there are some code cleanups mixed in here, too ... sorry, I couldn't help it.
> 
> 
> This addresses bug 13566.
>     http://bugs.digium.com/view.php?id=13566
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/include/asterisk/astobj2.h 166056 
>   /branches/1.4/include/asterisk/strings.h 166056 
>   /branches/1.4/main/astobj2.c 166056 
>   /branches/1.4/res/res_musiconhold.c 166056 
> 
> Diff: http://reviewboard.digium.com/r/106/diff
> 
> 
> Testing
> -------
> 
> I have made a bunch of calls to MoH.  I have done a bunch of reloads.  I did reloads adding and removing classes.  I also made sure that I did a reload to remove a class that was in use.
> 
> During these tests, I made sure that classes got destroyed when they were supposed to, and that there were no memory leaks.  All looks good.
> 
> 
> Thanks,
> 
> Russell
> 
>