[asterisk-dev] [Code Review] features reload does not clear old configuration and always include 700 on parkedcalls

[jrose]

> On 2011-04-05 17:24:34, David Vossel wrote:
> > /trunk/main/features.c, lines 4801-4817
> > <https://reviewboard.asterisk.org/r/1151/diff/1/?file=15995#file15995line4801>
> >
> >     If you take this out, what happens when parking lots other than the default are defined?

Aaaaaand... crap.  Back to the drawing board.


- jrose


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


On 2011-03-28 16:03:17, jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1151/
> -----------------------------------------------------------
> 
> (Updated 2011-03-28 16:03:17)
> 
> 
> Review request for Asterisk Developers, Russell Bryant and David Vossel.
> 
> 
> Summary
> -------
> 
> What I think is happening:
> everything is in features.c unless stated otherwise
> 
> within load_config
> default_parkinglot = build_parkinglot(...) is ran when there is not already a default_parkinglot, automatically creates context and extension using default data.  This is bad, because a little while later...
> the config file is loaded and data is populated for creating the context.
> The new context is created, so we have a second context in the dialplan which depending on config files may or may not be the same context.
> 
> Oh, and also we weren't deleting the contexts/extensions when we changed them.
> 
> fix:
> First, the context/extension is no longer created in the build_parkinglot method.  The wrong context is no longer made and this should be safe since we'll be creating the context soon after that in load_config anyway.
> 
> Second, if the default_parkinglot is not null when load_config is invoked (as in, by reload features in CLI), the context belonging to the default_parkinglot before the config file is reparsed gets cleared.
> 
> All in all, I think that should fix the problem.
> 
> There might be some other code in build_parkinglot which could do with some disposal.  I'm afraid to touch a lot of it right now since I don't know what all of it pertains to.  Also, anything that just writes to the parkinglot structure should probably be ignored since this just supplies default values.
> 
> 
> This addresses bug 18801.
>     https://issues.asterisk.org/view.php?id=18801
> 
> 
> Diffs
> -----
> 
>   /trunk/main/features.c 311734 
> 
> Diff: https://reviewboard.asterisk.org/r/1151/diff
> 
> 
> Testing
> -------
> 
> I tested the code as it was changed using features.conf and played with extension and context settings.  Tried a number of reload features commands and it doesn't seem to break from that.  Of course, I also changed features.conf between reloads and left it alone between some other reloads to check if it would behave properly when loading the same context it just destroyed.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110406/6ee4711b/attachment-0001.htm>