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

[Mark Michelson]

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


Let me add a disclaimer to the top of this review that I am not intimately familiar with the goings-on in the call parking code, so if I say something that is not true down below, please feel free to put me in my place and tell me how wrong I am.


/trunk/main/features.c
<https://reviewboard.asterisk.org/r/539/#comment3637>

    Is this safe? I ask because you will have args pointing to the pu that is created in park_space_reserve. I'm wondering if it is possible for manage_parkinglot to free this pointer out from under you.
    
    Or is such a problem protected by the (terribly-named) notquiteyet field of the pu struct?



/trunk/main/features.c
<https://reviewboard.asterisk.org/r/539/#comment3638>

    Is there a potential for multiple copies of args->pu to be in the list? If not, then you can break at this point instead of continuing.



/trunk/main/features.c
<https://reviewboard.asterisk.org/r/539/#comment3639>

    I'm a bit curious why you only unref the parkinglot and free args->pu if you find the pu in the parkinglot.
    
    Shouldn't you unref the parkinglot and free memory unconditionally?


- Mark


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
> 
>