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

[mlehner]

> 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
> 
> Tilghman Lesher wrote:
>     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).

The checks I removed caused the eventtype field to always be NULL when the column was an integer. This is not an issue of a default value, nor does this patch revert revision 236847. The extraneous checks for integer values were added (to the best of my searching) when the file was renamed from cel_adpative_odbc.c to cel_odbc.c. There is no comment in the SVN history as to why the extra checks were added at that time. My patch removes these checks and actually brings the code closer to revision 236847.


- mlehner


-----------------------------------------------------------
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/20110329/65e32d9f/attachment-0001.htm>