[Devel] memory leak and misc fixes to unixodbc
S G
skg1010 at hotmail.com
Thu Apr 6 20:01:45 CEST 2006
Hi
I've corrected some issues with unixodbc module leaking memory in the covert
rows function. I've also removed some deprecated ODBC functions, fixed
invalid cursort state on error and fixed error reporting from ODBC. The
module tests and works properly now. Attached is the patch.
-Sumeet Gupta
_________________________________________________________________
Don’t just search. Find. Check out the new MSN Search!
http://search.msn.click-url.com/go/onm00200636ave/direct/01/
-------------- next part --------------
cvs diff: Diffing modules/unixodbc-SG
Index: modules/unixodbc-SG/dbase.c
===================================================================
RCS file: /cvsroot/openser/sip-server/modules/unixodbc/dbase.c,v
retrieving revision 1.3
diff -u -r1.3 dbase.c
--- modules/unixodbc-SG/dbase.c 1 Mar 2006 10:13:37 -0000 1.3
+++ modules/unixodbc-SG/dbase.c 6 Apr 2006 17:40:24 -0000
@@ -25,6 +25,8 @@
* History:
* --------
* 2005-12-01 initial commit (chgen)
+ * 2006-04-03 fixed invalid handle to extract error (sgupta)
+ * 2006-04-04 removed deprecated ODBC functions, closed cursors on error
(sgupta)
*/
@@ -52,32 +54,80 @@
static int submit_query(db_con_t* _h, const char* _s)
{
int ret;
+ SQLCHAR outstr[1024];
+ SQLSMALLINT outstrlen;
+ SQLCHAR sqlstate[15];
+ SQLCHAR ErrorMsg[SQL_MAX_MESSAGE_LENGTH+1];
+ SQLINTEGER native_error = 0;
+ char conn_str[MAX_CONN_STR_LEN];
+ SQLRETURN sts;
/* first do some cleanup if required */
if(CON_RESULT(_h))
{
- SQLFreeStmt(&CON_RESULT(_h), SQL_RESET_PARAMS);
- SQLFreeStmt(&CON_RESULT(_h), SQL_UNBIND);
- SQLFreeStmt(&CON_RESULT(_h), SQL_CLOSE);
- SQLFreeStmt(&CON_RESULT(_h), SQL_DROP);
+ SQLCloseCursor(CON_RESULT(_h));
+ SQLFreeHandle(SQL_HANDLE_STMT, CON_RESULT(_h));
}
- ret = SQLAllocStmt(CON_CONNECTION(_h), &CON_RESULT(_h));
- if (!SQL_SUCCEEDED(ret))
- {
- LOG(L_ERR, "Statement allocation error %d\n",
- (int)(long)CON_CONNECTION(_h));
- extract_error("SQLAllocStmt", CON_CONNECTION(_h),
SQL_HANDLE_DBC);
- return ret;
- }
+ ret = SQLAllocHandle(SQL_HANDLE_STMT, CON_CONNECTION(_h),
&CON_RESULT(_h));
+ if (!SQL_SUCCEEDED(ret))
+ {
+ LOG(L_ERR, "ERROR:unixodbc:submit_query: Statement allocation
error %d\n",
+ (int)(long)CON_CONNECTION(_h));
+ extract_error("SQLAllocStmt", CON_CONNECTION(_h),
SQL_HANDLE_DBC);
+
+ sts = SQLError (CON_ENV(_h), CON_CONNECTION(_h), SQL_NULL_HSTMT,
sqlstate, &native_error,
+ ErrorMsg, sizeof(ErrorMsg), NULL);
+ if (SQL_SUCCEEDED (sts) &&
+ strncmp(sqlstate,"08003",5)!=0 && /* Connect not open */
+ strncmp(sqlstate,"08S01",5)!=0) /* Communication link
failure */
+ {
+ return ret;
+ }
+
+ LOG(L_ERR, "ERROR:unixodbc:submit_query: try to reconnect\n");
+
+ // Disconnect
+ SQLDisconnect (CON_CONNECTION(_h));
+
+ // Reconnect
+ if (!build_conn_str(CON_ID(_h), conn_str)) {
+ LOG(L_ERR, "ERROR:unixodbc:submit_query: failed to build
connection string\n");
+ return ret;
+ }
+
+ ret = SQLDriverConnect(CON_CONNECTION(_h), (void *)1,
(SQLCHAR*)conn_str,
+ SQL_NTS, outstr, sizeof(outstr),
&outstrlen,
+ SQL_DRIVER_COMPLETE);
+ if (!SQL_SUCCEEDED(ret)) {
+ LOG(L_ERR, "ERROR:unixodbc:submit_query: failed to
connect\n");
+ extract_error("SQLDriverConnect", CON_CONNECTION(_h),
SQL_HANDLE_DBC);
+ return ret;
+ }
+
+ //ret = SQLAllocStmt(CON_CONNECTION(_h), &CON_RESULT(_h));
+ ret = SQLAllocHandle(SQL_HANDLE_STMT, CON_CONNECTION(_h),
&CON_RESULT(_h));
+ if (!SQL_SUCCEEDED(ret))
+ {
+ LOG(L_ERR, "Statement allocation error %d\n",
+ (int)(long)CON_CONNECTION(_h));
+ extract_error("SQLAllocStmt", CON_CONNECTION(_h),
SQL_HANDLE_DBC);
+ return ret;
+ }
+ }
+
ret=SQLExecDirect(CON_RESULT(_h), (SQLCHAR*)_s, SQL_NTS);
if (!SQL_SUCCEEDED(ret))
{
- LOG(L_ERR, "Return value: %d\n", ret);
- extract_error("SQLExecDirect", CON_CONNECTION(_h),
SQL_HANDLE_DBC);
+ LOG(L_ERR, "SQLExecDirect, rv=%d. Query= %s\n", ret, _s);
+ extract_error("SQLExecDirect", CON_RESULT(_h),
SQL_HANDLE_STMT);
+
+ //Close the cursor
+ SQLCloseCursor(CON_RESULT(_h));
+ SQLFreeHandle(SQL_HANDLE_STMT, CON_RESULT(_h));
}
- return ret;
+ return ret;
}
/*
@@ -434,14 +484,14 @@
*(sql_buf + off) = '\0';
if (submit_query(_h, sql_buf) < 0)
{
- LOG(L_ERR, "db_query: Error while submitting query\n");
+ LOG(L_ERR, "unixodbc:db_query: Error while submitting
query\n");
return -2;
}
return store_result(_h, _r);
error:
- LOG(L_ERR, "db_query: Error in snprintf\n");
+ LOG(L_ERR, "unixodbc:db_query: Error in snprintf\n");
return -1;
}
@@ -600,9 +650,8 @@
ret = print_where(&CON_CONNECTION(_h), sql_buf + off,
SQL_BUF_LEN - off, _k, _o, _v, _n);
if (ret < 0) return -1;
off += ret;
-
- *(sql_buf + off) = '\0';
}
+ *(sql_buf + off) = '\0';
if (submit_query(_h, sql_buf) < 0)
{
Index: modules/unixodbc-SG/list.c
===================================================================
RCS file: /cvsroot/openser/sip-server/modules/unixodbc/list.c,v
retrieving revision 1.5
diff -u -r1.5 list.c
--- modules/unixodbc-SG/list.c 10 Jan 2006 10:49:18 -0000 1.5
+++ modules/unixodbc-SG/list.c 6 Apr 2006 17:40:24 -0000
@@ -25,86 +25,70 @@
* History:
* --------
* 2005-12-01 initial commit (chgen)
+ * 2006-04-04 simplified link list (sgupta)
*/
#include "../../dprint.h"
#include "../../mem/mem.h"
#include "list.h"
-int insert(list l, int n, strn* value)
+int insert(list** start, list** link, int n, strn* value)
{
- int i;
- list ln;
-
- if(!l->next)
- {
- ln=(list)pkg_malloc(sizeof(element));
- if(!ln)
- {
- LOG(L_ERR,"ERROR:unixodbc:insert: No enought pkg
memory (1)\n");
- return -1;
- }
- ln->data=pkg_malloc(sizeof(strn)*n);
- if(!ln->data)
- {
- LOG(L_ERR,"ERROR:unixodbc:insert: No enought pkg
memory (2)\n");
- pkg_free(ln);
- return -1;
- }
- for(i=0; i<n; i++)
- strcpy(ln->data[i].s, value[i].s);
- /* link it */
- ln->next=NULL;
- ln->end=ln;
- l->next=ln;
- l->end=ln;
-
- return 0;
+ int i = 0;
+
+ if(!(*start)) {
+ *link = (list*)pkg_malloc(sizeof(list));
+ if(!(*link)) {
+ LOG(L_ERR,"ERROR:unixodbc:insert: Not enough pkg memory
(1)\n");
+ return -1;
+ }
+ (*link)->next = NULL;
+ (*link)->data = pkg_malloc(sizeof(strn)*n);
+ if(!(*link)->data) {
+ LOG(L_ERR,"ERROR:unixodbc:insert: Not enough pkg memory
(2)\n");
+ pkg_free(*link);
+ *link = NULL;
+ return -1;
+ }
+ for(i=0; i<n; i++)
+ strcpy((*link)->data[i].s, value[i].s);
+
+ *start = *link;
+ return 0;
}
else
- return insert(l->end, n, value);
-}
-
-void destroy(list l)
-{
- if(l->next)
- destroy(l->next);
- pkg_free(l->data);
- pkg_free(l);
-}
+ {
+ list* nlink;
+ nlink=(list*)pkg_malloc(sizeof(list));
+ if(!nlink) {
+ LOG(L_ERR,"ERROR:unixodbc:insert: Not enough pkg memory (3)\n");
+ return -1;
+ }
+ nlink->data = pkg_malloc(sizeof(strn)*n);
+ if(!nlink->data) {
+ LOG(L_ERR,"ERROR:unixodbc:insert: Not enough pkg memory (4)\n");
+ pkg_free(nlink);
+ return -1;
+ }
-strn* view(list l)
-{
- return l->data;
+ for(i=0; i<n; i++)
+ strcpy(nlink->data[i].s, value[i].s);
+
+ nlink->next = NULL;
+ (*link)->next = nlink;
+ *link = (*link)->next;
+
+ return 0;
+ }
}
-int create(list *l, int n, strn* value)
+void destroy(list *start)
{
- int i;
- if(*l)
- {
- LOG(L_WARN,"WARNING:unixodbc:create: List already
created\n");
- return 0;
- }
- *l=(list)pkg_malloc(sizeof(element));
- if(!*l)
- {
- LOG(L_ERR,"ERROR:unixodbc:create: No enought pkg memory
(1)\n");
- return -1;
- }
- (*l)->next=NULL;
- (*l)->end=*l;
- (*l)->data=pkg_malloc(sizeof(strn)*n);
- if(!(*l)->data)
- {
- LOG(L_ERR,"ERROR:unixodbc:create: No enought pkg memory
(2)\n");
- pkg_free(*l);
- *l = 0;
- return -1;
- }
- for(i=0; i<n; i++)
- strcpy((*l)->data[i].s, value[i].s);
-
- return 0;
+ while(start) {
+ list* temp = start;
+ start = start->next;
+ pkg_free(temp->data);
+ pkg_free(temp);
+ }
}
Index: modules/unixodbc-SG/list.h
===================================================================
RCS file: /cvsroot/openser/sip-server/modules/unixodbc/list.h,v
retrieving revision 1.2
diff -u -r1.2 list.h
--- modules/unixodbc-SG/list.h 10 Jan 2006 10:49:18 -0000 1.2
+++ modules/unixodbc-SG/list.h 6 Apr 2006 17:40:24 -0000
@@ -25,6 +25,7 @@
* History:
* --------
* 2005-12-01 initial commit (chgen)
+ * 2006-04-04 simplified link list (sgupta)
*/
#ifndef _UNIXODBC_LIST_H_
@@ -34,23 +35,14 @@
#include <stdlib.h>
#include "my_con.h"
-typedef struct element
+typedef struct list
{
- struct element *next;
- struct element *end;
-
+ struct list *next;
strn *data;
-} element;
-
-typedef element* list;
-
-int insert(list l, int n, strn* value);
-
-void destroy(list l);
-
-strn* view(list l);
+} list;
-int create(list *l, int n, strn* value);
+int insert(list** start, list** link, int n, strn* value);
+void destroy(list* link);
#endif
Index: modules/unixodbc-SG/my_con.c
===================================================================
RCS file: /cvsroot/openser/sip-server/modules/unixodbc/my_con.c,v
retrieving revision 1.4
diff -u -r1.4 my_con.c
--- modules/unixodbc-SG/my_con.c 1 Mar 2006 10:13:37 -0000 1.4
+++ modules/unixodbc-SG/my_con.c 6 Apr 2006 17:40:24 -0000
@@ -43,14 +43,14 @@
#define PWD_ATTR "PWD="
#define PWD_ATTR_LEN (sizeof(PWD_ATTR)-1)
-#define MAX_CONN_STR_LEN 2048
-static char *build_conn_str(struct db_id* id)
+char *build_conn_str(struct db_id* id, char *buf)
{
- static char buf[MAX_CONN_STR_LEN];
int len, ld, lu, lp;
char *p;
+ if (!buf) return 0;
+
ld = id->database?strlen(id->database):0;
lu = id->username?strlen(id->username):0;
lp = id->password?strlen(id->password):0;
@@ -104,7 +104,7 @@
SQLSMALLINT outstrlen;
int ret;
struct my_con* ptr;
- char *conn_str;
+ char conn_str[MAX_CONN_STR_LEN];
if (!id)
{
@@ -126,8 +126,7 @@
SQLSetEnvAttr(ptr->env, SQL_ATTR_ODBC_VERSION, (void *)
SQL_OV_ODBC3, 0);
SQLAllocHandle(SQL_HANDLE_DBC, ptr->env, &(ptr->dbc));
- conn_str = build_conn_str(id);
- if (conn_str==0) {
+ if (!build_conn_str(id, conn_str)) {
LOG(L_ERR, "ERROR:unixodbc:new_connection: failed to build "
"connection string\n");
return 0;
@@ -189,15 +188,11 @@
SQLSMALLINT len;
SQLRETURN ret;
- LOG(L_ERR,"ERROR:unixodbc: the driver reported the following
diagnostics "
- "whilst running %s\n",fn);
-
do
{
- ret = SQLGetDiagRec(type, handle, ++i, state, &native, text,
- sizeof(text), &len );
+ ret = SQLGetDiagRec(type, handle, ++i, state, &native, text,
sizeof(text), &len );
if (SQL_SUCCEEDED(ret))
- LOG(L_ERR,"\t%s:%ld:%ld:%s\n", state, (long)i,
(long)native, text);
+ LOG(L_ERR,"unixodbc:SQLGetDiagRec=%s:%ld:%ld:%s\n",
state, (long)i, (long)native, text);
}
while( ret == SQL_SUCCESS );
}
Index: modules/unixodbc-SG/my_con.h
===================================================================
RCS file: /cvsroot/openser/sip-server/modules/unixodbc/my_con.h,v
retrieving revision 1.3
diff -u -r1.3 my_con.h
--- modules/unixodbc-SG/my_con.h 6 Mar 2006 09:00:40 -0000 1.3
+++ modules/unixodbc-SG/my_con.h 6 Apr 2006 17:40:24 -0000
@@ -66,6 +66,10 @@
#define CON_CONNECTION(db_con) (((struct my_con*)((db_con)->tail))->dbc)
#define CON_ROW(db_con) (((struct
my_con*)((db_con)->tail))->row)
#define CON_TIMESTAMP(db_con) (((struct
my_con*)((db_con)->tail))->timestamp)
+#define CON_ID(db_con) (((struct my_con*)((db_con)->tail))->id)
+#define CON_ENV(db_con) (((struct my_con*)((db_con)->tail))->env)
+#define MAX_CONN_STR_LEN 2048
+
/*
* Create a new connection structure,
@@ -76,7 +80,12 @@
/*
* Close the connection and release memory
*/
+
void free_connection(struct my_con* con);
void extract_error(char *fn, SQLHANDLE handle, SQLSMALLINT type);
-#endif
/* MY_CON_H */
+
+char *build_conn_str(struct db_id* id, char *buf);
+
+
+#endif /* MY_CON_H */
Index: modules/unixodbc-SG/res.c
===================================================================
RCS file: /cvsroot/openser/sip-server/modules/unixodbc/res.c,v
retrieving revision 1.3
diff -u -r1.3 res.c
--- modules/unixodbc-SG/res.c 1 Mar 2006 10:13:37 -0000 1.3
+++ modules/unixodbc-SG/res.c 6 Apr 2006 17:40:24 -0000
@@ -25,6 +25,7 @@
* History:
* --------
* 2005-12-01 initial commit (chgen)
+ * 2006-04-04 fixed memory leak in convert_rows (sgupta)
*/
#include "../../mem/mem.h"
@@ -46,28 +47,28 @@
if ((!_h) || (!_r))
{
- LOG(L_ERR, "get_columns: Invalid parameter\n");
+ LOG(L_ERR, "unixodbc:get_columns: Invalid parameter\n");
return -1;
}
SQLNumResultCols(CON_RESULT(_h), &n);
if (!n)
{
- LOG(L_ERR, "get_columns: No columns\n");
+ LOG(L_ERR, "unixodbc:get_columns: No columns\n");
return -2;
}
-
+
RES_NAMES(_r) = (db_key_t*)pkg_malloc(sizeof(db_key_t) * n);
if (!RES_NAMES(_r))
{
- LOG(L_ERR, "get_columns: No memory left\n");
+ LOG(L_ERR, "unixodbc:get_columns: No memory left\n");
return -3;
}
RES_TYPES(_r) = (db_type_t*)pkg_malloc(sizeof(db_type_t) * n);
if (!RES_TYPES(_r))
{
- LOG(L_ERR, "get_columns: No memory left\n");
+ LOG(L_ERR, "unixodbc:get_columns: No memory left\n");
pkg_free(RES_NAMES(_r));
return -4;
}
@@ -131,7 +132,7 @@
if (!_r)
{
- LOG(L_ERR, "free_rows: Invalid parameter value\n");
+ LOG(L_ERR, "unixodbc:free_rows: Invalid parameter value\n");
return -1;
}
@@ -148,93 +149,89 @@
*/
static inline int convert_rows(db_con_t* _h, db_res_t* _r)
{
- int row, i, ret;
+ int row_n = 0, i = 0, ret = 0;
SQLSMALLINT columns;
- list rows = NULL;
- list l;
+ list* rows = NULL;
+ list* rowstart = NULL;
if((!_h) || (!_r))
{
- LOG(L_ERR, "convert_rows: Invalid parameter\n");
+ LOG(L_ERR, "unixodbc:convert_rows: Invalid parameter\n");
return -1;
}
- row = 0;
SQLNumResultCols(CON_RESULT(_h), (SQLSMALLINT *)&columns);
CON_ROW(_h) = (strn*)pkg_malloc( columns*sizeof(strn) );
if(!CON_ROW(_h))
{
- LOG(L_ERR, "convert_rows: No memory left\n");
+ LOG(L_ERR, "unixodbc:convert_rows: No memory left\n");
return -1;
}
while(SQL_SUCCEEDED(ret = SQLFetch(CON_RESULT(_h))))
{
- for(i=1; i <= columns; i++)
+ for(i=0; i < columns; i++)
{
SQLINTEGER indicator;
- ret = SQLGetData(CON_RESULT(_h), i, SQL_C_CHAR,
- (CON_ROW(_h)[i-1]).s, STRN_LEN, &indicator);
- if (SQL_SUCCEEDED(ret))
- {
- if (indicator == SQL_NULL_DATA)
strcpy((CON_ROW(_h)[i-1]).s, "NULL");
+ ret = SQLGetData(CON_RESULT(_h), i+1, SQL_C_CHAR,
+ (CON_ROW(_h)[i]).s, STRN_LEN, &indicator);
+ if (SQL_SUCCEEDED(ret)) {
+ if (indicator == SQL_NULL_DATA)
strcpy((CON_ROW(_h)[i]).s, "NULL");
}
- else
- {
- LOG(L_ERR, "Error in SQLGetData\n");
+ else {
+ LOG(L_ERR, "unixodbc:convert_rows: Error in
SQLGetData\n");
}
}
- if(!row)
- {
- create(&rows, columns, CON_ROW(_h));
+
+ if (insert(&rowstart, &rows, columns, CON_ROW(_h)) < 0) {
+ LOG(L_ERR, "unixodbc:convertrows: insert failed\n");
+ pkg_free(CON_ROW(_h));
+ CON_ROW(_h) = NULL;
+ return -5;
}
- else
- {
- insert(rows, columns, CON_ROW(_h));
- }
- row++;
+
+ row_n++;
}
+ //free temporary CON_ROW(_h) data
+ pkg_free(CON_ROW(_h));
+ CON_ROW(_h) = NULL;
- RES_ROW_N(_r) = row;
- if (!row)
+ RES_ROW_N(_r) = row_n;
+ if (!row_n)
{
RES_ROWS(_r) = 0;
return 0;
}
- RES_ROWS(_r) = (struct db_row*)pkg_malloc(sizeof(db_row_t) * row);
+ RES_ROWS(_r) = (struct db_row*)pkg_malloc(sizeof(db_row_t) * row_n);
if (!RES_ROWS(_r))
{
- LOG(L_ERR, "convert_rows: No memory left\n");
+ LOG(L_ERR, "unixodbc:convert_rows: No memory left\n");
return -2;
}
i = 0;
- l = rows;
- while(l!=NULL)
+ rows = rowstart;
+ while(rows)
{
- CON_ROW(_h) = view(l);
- //for(j=0; j<columns; j++)
- //{
- //if(!strcmp(CON_ROW(_h)[j].s, "NULL"))
- // strcpy(CON_ROW(_h)[j].s, "");
- //}
+ CON_ROW(_h) = rows->data;
if (!CON_ROW(_h))
{
- LOG(L_ERR, "convert_rows: string null\n");
- RES_ROW_N(_r) = row;
+ LOG(L_ERR, "unixodbc:convert_rows: string null\n");
+ RES_ROW_N(_r) = row_n;
free_rows(_r);
return -3;
}
if (convert_row(_h, _r, &(RES_ROWS(_r)[i])) < 0)
{
- LOG(L_ERR, "convert_rows: Error while converting row
#%d\n", i);
+ LOG(L_ERR, "unixodbc:convert_rows: Error while
converting row #%d\n", i);
RES_ROW_N(_r) = i;
free_rows(_r);
return -4;
}
i++;
- l=l->next;
+ rows = rows->next;
}
- destroy(rows);
+
+ destroy(rowstart);
return 0;
}
cvs diff: Diffing modules/unixodbc-SG/doc
More information about the Devel
mailing list