[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] cdr_odbc.c is broken in trunk
From:       Nick Gorham <nick () lurcher ! org>
Date:       2008-01-11 11:34:44
Message-ID: 47875454.6020207 () lurcher ! org
[Download RAW message or body]

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

            

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.com--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]