[asterisk-dev] [Code Review] Clean up func_odbc's dummy channel usage.

Tilghman Lesher tlesher at digium.com
Tue Jun 23 21:49:19 CDT 2009


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


While I kind of understand why you're proposing this change, I'd really prefer that you did it a different way.  The method that I used was primarily to help avoid bugs by ensuring that the channel exists at all times throughout the execution.  If you wanted to eliminate the autoservice and the setting of variables on a bogus channel, I think that would be a better way to go, but the bogus channel existence I believe should be kept for the duration.  The only benefit I see there is a temporary easing of memory usage, which isn't that great, anyway.  I think the consistency of having a channel, which helps avoid future bugs is a better way to go.


/trunk/funcs/func_odbc.c
<http://reviewboard.digium.com/r/290/#comment2125>

    I'm not really sold on this change.  Add an else clause, if you want to handle the error, instead of changing how this works.


- Tilghman


On 2009-06-23 17:54:19, Matthew Nicholson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/290/
> -----------------------------------------------------------
> 
> (Updated 2009-06-23 17:54:19)
> 
> 
> Review request for Asterisk Developers and Tilghman Lesher.
> 
> 
> Summary
> -------
> 
> This patch cleans up func_odbc's use of dummy channels a bit.  This is accomplished by releasing the dummy channel after we no longer need it and skipping sections of code that are pointless to execute without a channel.
> 
> 
> Diffs
> -----
> 
>   /trunk/funcs/func_odbc.c 202792 
> 
> Diff: http://reviewboard.digium.com/r/290/diff
> 
> 
> Testing
> -------
> 
> None.
> 
> 
> Thanks,
> 
> Matthew
> 
>




More information about the asterisk-dev mailing list