[asterisk-dev] cdr_odbc.c is broken in trunk

Tilghman Lesher tilghman at mail.jeffandtilghman.com
Fri Jan 11 09:06:32 CST 2008


On Friday 11 January 2008 05:34:44 Nick Gorham wrote:
> Kevin P. Fleming wrote:
> >Tilghman Lesher wrote:
> >>Uh, no, the stack doesn't work that way.  The entire contents of
> >> timestr[] remain valid and unchanged until the stack frame is popped. 
> >> Which will not happen until odbc_log() exits.  The only reason this
> >> location would be overwritten is if there's a stack overflow error (not
> >> impossible, but we've been fairly diligent in finding those issues).
> >
> >This is incorrect. The variable being referred to here ('timestr' in
> >prepare_cb) is allocated on the stack when prepare_cb() is entered and
> >then that stack frame is released when prepare_cb() exits. Since
> >ast_odbc_prepare_and_execute() calls prepare_cb() and then later calls
> >SQLExecute(), the memory for this bound parameter will now be used for
> >something else (probably the stack frame for SQLExecute() itself).
>
> Given the hassle about me posting the patch, and given the licence is
> still pending, I guess that no one looked at my patch yet?

Your license was still pending at the time of commit, so no, I have not looked
at it.

> The reason I ask, is getting the current trunk I see the code creates
> the query using
>
>             "VALUES ('%s',?,?,?,?,?,?,?,?,?,?,?,?,?)", table, timestr);
>
> My version used the ODBC timestamp escape syntax, which I think you will
> find is a better choice than assuming the ODBC driver in use will
> understand the format from ast_strftime, using the ODBC escape means
> that the driver will understand {ts 'YYYY-MM-DD HH:MM:SS' }if it doesn't
> then at least the spec says it should. I used
>
>             "VALUES ({ts '%s'},?,?,?,?,?,?,?,?,?,?,?,?,?)", table,
> timestr);
>
> Not a big deal, but I don't think it hurts to use the standard when
> there is one avalable.

Do you have an example of a database driver that does not work with the
current syntax?

-- 
Tilghman



More information about the asterisk-dev mailing list