[asterisk-dev] [Code Review] extra ast_strlen_zero() check in cel_odbc prevents integer eventtype in database

Tilghman Lesher reviewboard at asterisk.org
Sat Mar 26 17:33:53 CDT 2011



> On 2011-03-26 17:12:24, Tilghman Lesher wrote:
> > This change reverts revision 236847, which was reported on the -dev list by Nic Colledge.  You need to address his comments, before this can go forward:
> > http://lists.digium.com/pipermail/asterisk-dev/2009-December/041362.html

It's probably better for you to specify a default in your table, instead of making a source code change.  A blank is almost never a good option for an enum value (especially as it applies to Asterisk).


- Tilghman


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


On 2011-03-26 11:43:38, mlehner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1149/
> -----------------------------------------------------------
> 
> (Updated 2011-03-26 11:43:38)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> event_type could not be an integer datatype in a ODBC database because an extra ast_strlen_zero() check was wrapped around the code that did the work. ast_strlen_zero() check is done in the second branch of the if statement, after the check for eventtype is done. Changes are very simple, just removing the "overzealous" ast_strlen_zero() check, while keeping braces to prevent compilation errors and warnings with variable re-definitions. 
> 
> 
> Diffs
> -----
> 
>   /tags/1.8.3.2/cel/cel_odbc.c 311686 
> 
> Diff: https://reviewboard.asterisk.org/r/1149/diff
> 
> 
> Testing
> -------
> 
> Tested on a local install of Asterisk 1.8.3.2. Compiles and creates the expected records in the database.
> 
> 
> Thanks,
> 
> mlehner
> 
>

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


More information about the asterisk-dev mailing list