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

Nick Gorham nick at lurcher.org
Fri Jan 11 05:34:44 CST 2008


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?

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.

-- 
Nick Gorham

            



More information about the asterisk-dev mailing list