[asterisk-dev] [Code Review] Park() Application fixes to R (randomize parkinglot extension) option

[David Vossel]

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

Ship it!


Great work! The code looks correct, as long as this has been tested both using and not using the 'R' option this should be good to go.

- David


On 2011-05-19 11:17:09, jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1222/
> -----------------------------------------------------------
> 
> (Updated 2011-05-19 11:17:09)
> 
> 
> Review request for Asterisk Developers and David Vossel.
> 
> 
> Summary
> -------
> 
> Deals with some issues with the randomize option in Park().  Parking would result in a low starting extension ranging from 0 to the difference in the parkinglot's last extension and first extension.  This was just an issue of someone forgetting to add the start value to the random value.  After that I found that if it wrapped around before selecting a parking extension, and if the chosen parking extension from the random wasn't just the first extension in the available extensions list, the parking function would get mess up and the parking extension would be set to -1 which was allowed for some reason.
> 
> To address this, I made it so that if randomize mode is enabled, and if the random parking lot picked wasn't the first one in the list, a loop value would be raised so that the exit conditions would be able to tell if there were unchecked parking extensions and start iterating from the beginning if it reached that point.  Now all extensions get checked and there is no longer an opportunity for a false exit of the loop.
> 
> This does have a small performance impact with a worse case scenario of adding n-1 additional extension checks where n is the number of parking extensions available in the parking lot.  This will only be the case if the parking lot is full and the chosen parking extension from the random value is the one immediately after the first.  It could be done better possibly by holding onto the first extension checked so that the loop could be terminated early if it reached that repetition point.  This would require extra evaluations during every step though, so the performance increase might be non-existent, and even if it isn't this is very much one of those 'once in a while' situations where we probably don't care if it takes a little extra time to evaluate.
> 
> 
> This addresses bug 18862.
>     https://issues.asterisk.org/view.php?id=18862
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/features.c 319525 
> 
> Diff: https://reviewboard.asterisk.org/r/1222/diff
> 
> 
> Testing
> -------
> 
> Simple 3 slot parking check.  Nothing to write home about.  Observed with multiple different starting values for behavior.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110519/7324beae/attachment.htm>