[asterisk-dev] [Code Review] Fix for parking lot settings not being respected with unit test

[Jeff Peeler]

> On 2010-03-03 13:44:10, David Vossel wrote:
> > /trunk/main/features.c, lines 1173-1186
> > <https://reviewboard.asterisk.org/r/539/diff/1/?file=8461#file8461line1173>
> >
> >     This enables parkeddynamic when the test initializes and does not revert that change.  This code should go below the switch case.

Oops, thanks.


> On 2010-03-03 13:44:10, David Vossel wrote:
> > /trunk/main/features.c, line 1157
> > <https://reviewboard.asterisk.org/r/539/diff/1/?file=8461#file8461line1157>
> >
> >     I could very well be missing something here, but it is not obvious to me how this channel is hungup during some error cases.  It seems like it might be good to have a
> >     
> >     if (test_channel1) {
> >         ast_hangup(test_channel1);
> >     }
> >     
> >     at the cleanup label.
> >     
> >     This would mean that whenever we know the channel has been hungup it would need to be set to NULL in the test function so it wouldn't try and hang it up twice.

Well, it is a bit confusing. Each time the channel is unparked, it is also hung up. I didn't bother setting the channel to NULL because I'm reusing that pointer for the next parking scenario. Maybe that is a bad idea?


- Jeff


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


On 2010-03-02 20:47:45, Jeff Peeler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/539/
> -----------------------------------------------------------
> 
> (Updated 2010-03-02 20:47:45)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This verifies a change in parking to ensure the channel variable PARKINGLOT is respected. Kind of overkill for an obvious change, but many more parking scenarios can be added later.
> 
> 
> This addresses bug 16592.
>     https://issues.asterisk.org/view.php?id=16592
> 
> 
> Diffs
> -----
> 
>   /trunk/main/features.c 250288 
> 
> Diff: https://reviewboard.asterisk.org/r/539/diff
> 
> 
> Testing
> -------
> 
> Verified proposed fix works and that all created channels are properly destroyed. It seems though that I do actually have a ref count problem with the dynamic lot used for testing, which I haven't been able to track down yet. I went ahead and posted the review so the rest of the test can be examined.
> 
> 
> Thanks,
> 
> Jeff
> 
>