[asterisk-dev] [Code Review] CEL cel_odbc backend fails to insert eventtype into integer or other numeric column

[kkm]

> On 2011-04-03 22:39:40, Tilghman Lesher wrote:
> > /trunk/cel/cel_odbc.c, line 574
> > <https://reviewboard.asterisk.org/r/1160/diff/2/?file=16055#file16055line574>
> >
> >     Why is this changing from 15 to 16?  Maximum length of the format string is still 15.

Yes, 15 here, with spaces removed.

Just wondering, what is the point of using these macros here? ast_str_append() extends the string automatically anyway. Is that so we can check for memory allocation errors?


> On 2011-04-03 22:39:40, Tilghman Lesher wrote:
> > /trunk/cel/cel_odbc.c, line 548
> > <https://reviewboard.asterisk.org/r/1160/diff/2/?file=16055#file16055line548>
> >
> >     This value should not have decremented to 15, as the format string still has a maximum result length of 17.

Yuck. 17. I removed the spaces just inside {} but did not recount.


> On 2011-04-03 22:39:40, Tilghman Lesher wrote:
> > /trunk/cel/cel_odbc.c, line 562
> > <https://reviewboard.asterisk.org/r/1160/diff/2/?file=16055#file16055line562>
> >
> >     Why is this necessary?

Leap second. At least, linux and bsd systems have this in /usr/include/time.h:

struct tm
{
  int tm_sec;                   /* Seconds.     [0-60] (1 leap second) */


> On 2011-04-03 22:39:40, Tilghman Lesher wrote:
> > /trunk/cel/cel_odbc.c, line 526
> > <https://reviewboard.asterisk.org/r/1160/diff/2/?file=16055#file16055line526>
> >
> >     usegmtime?

Good point. Missed that. Will fix, thanks. (x2)


- kkm


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


On 2011-04-03 22:16:18, kkm wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1160/
> -----------------------------------------------------------
> 
> (Updated 2011-04-03 22:16:18)
> 
> 
> Review request for Asterisk Developers and Tilghman Lesher.
> 
> 
> Summary
> -------
> 
> If eventtype is numeric in the database, it gets set to NULL, not to one of the numeric values as documented in cel_odbc.conf.sample.
> 
> 
> This addresses bug 18964.
>     https://issues.asterisk.org/view.php?id=18964
> 
> 
> Diffs
> -----
> 
>   /trunk/cel/cel_odbc.c 312554 
> 
> Diff: https://reviewboard.asterisk.org/r/1160/diff
> 
> 
> Testing
> -------
> 
> After preproduction testing, the change deployed to large scale production since March, 21.
> 
> 
> Thanks,
> 
> kkm
> 
>

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