[asterisk-dev] cdr_odbc.c is broken in trunk
Nick Gorham
nick at lurcher.org
Thu Jan 10 08:06:20 CST 2008
Michiel van Baak wrote:
>On 11:18, Thu 10 Jan 08, Nick Gorham wrote:
>
>
>>Hi,
>>
>>The change to use prepare_cb() in odbc_log became broken in r88182.
>>
>>The code calls SQLBindParameter on a local variable (timestr) but as it
>>now doesn't call SQLExecute in the same function when it finally gets to
>>SQLExecute that variable is out of scope.
>>
>>I dont know if I can pass attachments to this list, so the fixed cdr can
>>be found here http://www.lurcher.org/~nick/cdr_odbc.c
>>
>>The code contained the following lines
>>
>> /* We really should only have to do this once. But for some
>> * strange reason if I don't it blows holes in memory like
>> * like a shotgun. So we just do this so its safe.
>> */
>>
>>The "strange reason", as I have tried to report several times now via
>>the forum, is that the memory passed to SQLBindParameter is not
>>referenced until the SQLExecute, that the point of binding parameters,
>>you pass the address of the storage, then before the SQLExecute you set
>>the values you intend to use.
>>
>>
>
>Hi,
>
>Can you post this to http://bugs.digium.com with a unified
>diff as the patch ?
>Thank you.
>
>
>
Err, no as it happens, I don't see why I should provide anyone my
address and phone number to submit a bug fix to what I believed was a
GPL project!!!
Here is the patch in question.
--- cdr_odbc.c.orig 2008-01-10 05:01:29.000000000 -0500
+++ cdr_odbc.c 2008-01-10 06:21:53.000000000 -0500
@@ -74,12 +74,12 @@
snprintf(sqlcmd,sizeof(sqlcmd),"INSERT INTO %s "
"(calldate,clid,src,dst,dcontext,channel,dstchannel,lastapp,"
"lastdata,duration,billsec,disposition,amaflags,accountcode,uniqueid,userfield)
"
- "VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", table);
+ "VALUES ({ts '%s'},?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", table, timestr);
} else {
snprintf(sqlcmd,sizeof(sqlcmd),"INSERT INTO %s "
"(calldate,clid,src,dst,dcontext,channel,dstchannel,lastapp,lastdata,"
"duration,billsec,disposition,amaflags,accountcode) "
- "VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?)", table);
+ "VALUES ({ts '%s'},?,?,?,?,?,?,?,?,?,?,?,?,?)", table, timestr);
}
ODBC_res = SQLAllocHandle(SQL_HANDLE_STMT, obj->con, &stmt);
@@ -98,27 +98,39 @@
return NULL;
}
- SQLBindParameter(stmt, 1, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(timestr), 0, ×tr, 0, NULL);
- SQLBindParameter(stmt, 2, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->clid), 0, cdr->clid, 0, NULL);
- SQLBindParameter(stmt, 3, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->src), 0, cdr->src, 0, NULL);
- SQLBindParameter(stmt, 4, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->dst), 0, cdr->dst, 0, NULL);
- SQLBindParameter(stmt, 5, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->dcontext), 0, cdr->dcontext, 0, NULL);
- SQLBindParameter(stmt, 6, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->channel), 0, cdr->channel, 0, NULL);
- SQLBindParameter(stmt, 7, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->dstchannel), 0, cdr->dstchannel, 0, NULL);
- SQLBindParameter(stmt, 8, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->lastapp), 0, cdr->lastapp, 0, NULL);
- SQLBindParameter(stmt, 9, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->lastdata), 0, cdr->lastdata, 0, NULL);
- SQLBindParameter(stmt, 10, SQL_PARAM_INPUT, SQL_C_SLONG,
SQL_INTEGER, 0, 0, &cdr->duration, 0, NULL);
- SQLBindParameter(stmt, 11, SQL_PARAM_INPUT, SQL_C_SLONG,
SQL_INTEGER, 0, 0, &cdr->billsec, 0, NULL);
+ /*
+ * You can't do this, the bound parameter is only referenced when
the SQLExecute is called
+ * so by then the memory pointed to by timestr is no longer in
scope, and soit might work, it might
+ * insert whatever it finds, or it might seg fault, as the lack of a
length variable means it might go
+ * and look for a NULL, and given the memory is no longer in use as
a string, it may fail finding one
+ */
+
+ /* SQLBindParameter(stmt, 1, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(timestr), 0, ×tr, 0, NULL); */
+
+ /*
+ * because of the above, pass the first parameter as a string, so we
need to adjust the other parameter numbers
+ */
+
+ SQLBindParameter(stmt, 1, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->clid), 0, cdr->clid, 0, NULL);
+ SQLBindParameter(stmt, 2, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->src), 0, cdr->src, 0, NULL);
+ SQLBindParameter(stmt, 3, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->dst), 0, cdr->dst, 0, NULL);
+ SQLBindParameter(stmt, 4, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->dcontext), 0, cdr->dcontext, 0, NULL);
+ SQLBindParameter(stmt, 5, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->channel), 0, cdr->channel, 0, NULL);
+ SQLBindParameter(stmt, 6, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->dstchannel), 0, cdr->dstchannel, 0, NULL);
+ SQLBindParameter(stmt, 7, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->lastapp), 0, cdr->lastapp, 0, NULL);
+ SQLBindParameter(stmt, 8, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->lastdata), 0, cdr->lastdata, 0, NULL);
+ SQLBindParameter(stmt, 9, SQL_PARAM_INPUT, SQL_C_SLONG, SQL_INTEGER,
0, 0, &cdr->duration, 0, NULL);
+ SQLBindParameter(stmt, 10, SQL_PARAM_INPUT, SQL_C_SLONG,
SQL_INTEGER, 0, 0, &cdr->billsec, 0, NULL);
if (ast_test_flag(&config, CONFIG_DISPOSITIONSTRING))
- SQLBindParameter(stmt, 12, SQL_PARAM_INPUT, SQL_C_CHAR,
SQL_CHAR, strlen(ast_cdr_disp2str(cdr->disposition)) + 1, 0,
ast_cdr_disp2str(cdr->disposition), 0, NULL);
+ SQLBindParameter(stmt, 11, SQL_PARAM_INPUT, SQL_C_CHAR,
SQL_CHAR, strlen(ast_cdr_disp2str(cdr->disposition)) + 1, 0,
ast_cdr_disp2str(cdr->disposition), 0, NULL);
else
- SQLBindParameter(stmt, 12, SQL_PARAM_INPUT, SQL_C_SLONG,
SQL_INTEGER, 0, 0, &cdr->disposition, 0, NULL);
- SQLBindParameter(stmt, 13, SQL_PARAM_INPUT, SQL_C_SLONG,
SQL_INTEGER, 0, 0, &cdr->amaflags, 0, NULL);
- SQLBindParameter(stmt, 14, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->accountcode), 0, cdr->accountcode, 0, NULL);
+ SQLBindParameter(stmt, 11, SQL_PARAM_INPUT, SQL_C_SLONG,
SQL_INTEGER, 0, 0, &cdr->disposition, 0, NULL);
+ SQLBindParameter(stmt, 12, SQL_PARAM_INPUT, SQL_C_SLONG,
SQL_INTEGER, 0, 0, &cdr->amaflags, 0, NULL);
+ SQLBindParameter(stmt, 13, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR,
sizeof(cdr->accountcode), 0, cdr->accountcode, 0, NULL);
if (ast_test_flag(&config, CONFIG_LOGUNIQUEID)) {
- SQLBindParameter(stmt, 15, SQL_PARAM_INPUT, SQL_C_CHAR,
SQL_CHAR, sizeof(cdr->uniqueid), 0, cdr->uniqueid, 0, NULL);
- SQLBindParameter(stmt, 16, SQL_PARAM_INPUT, SQL_C_CHAR,
SQL_CHAR, sizeof(cdr->userfield), 0, cdr->userfield, 0, NULL);
+ SQLBindParameter(stmt, 14, SQL_PARAM_INPUT, SQL_C_CHAR,
SQL_CHAR, sizeof(cdr->uniqueid), 0, cdr->uniqueid, 0, NULL);
+ SQLBindParameter(stmt, 15, SQL_PARAM_INPUT, SQL_C_CHAR,
SQL_CHAR, sizeof(cdr->userfield), 0, cdr->userfield, 0, NULL);
}
return stmt;
--
Nick Gorham
More information about the asterisk-dev
mailing list