[sr-dev] git:4.0: db_mysql: fix segfault when recursive queries are made

Module: sip-router Branch: 4.0 Commit: 5e5ba673fa4d2351047eadeaec655017c962424d URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5e5ba67… Author: Richard Fuchs &lt;rfuchs(a)sipwise.com&gt; Committer: Richard Fuchs &lt;rfuchs(a)sipwise.com&gt; Date: Wed Apr 10 09:34:49 2013 -0400 db_mysql: fix segfault when recursive queries are made The MySQL result object (MYSQL_RES) should not be stored within the srdb1 connection object, but rather within the srdb1 result object. Otherwise recursive queries overwrite each other's result sets, which results in segfault. Conflicts: modules/db_mysql/km_my_con.h --- modules/db_mysql/km_dbase.c | 36 +++++++++++++++++++----------------- modules/db_mysql/km_dbase.h | 2 +- modules/db_mysql/km_my_con.c | 1 - modules/db_mysql/km_my_con.h | 4 ---- modules/db_mysql/km_res.c | 27 +++++++++++++++++++++++---- modules/db_mysql/km_res.h | 19 +++++++++++++++++++ modules/db_mysql/km_row.c | 5 +++-- 7 files changed, 65 insertions(+), 29 deletions(-) diff --git a/modules/db_mysql/km_dbase.c b/modules/db_mysql/km_dbase.c index e1cce08..151a24a 100644 --- a/modules/db_mysql/km_dbase.c +++ b/modules/db_mysql/km_dbase.c @@ -163,14 +163,14 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r) return -1; } - *_r = db_new_result(); + *_r = db_mysql_new_result(); if (*_r == 0) { LM_ERR("no memory left\n"); return -2; } - CON_RESULT(_h) = mysql_store_result(CON_CONNECTION(_h)); - if (!CON_RESULT(_h)) { + RES_RESULT(*_r) = mysql_store_result(CON_CONNECTION(_h)); + if (!RES_RESULT(*_r)) { if (mysql_field_count(CON_CONNECTION(_h)) == 0) { (*_r)->col.n = 0; (*_r)->n = 0; @@ -181,7 +181,7 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r) if (code == CR_SERVER_GONE_ERROR || code == CR_SERVER_LOST) { counter_inc(mysql_cnts_h.driver_err); } - db_free_result(*_r); + db_mysql_free_result(_h, *_r); *_r = 0; return -3; } @@ -195,14 +195,14 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r) /* all mem on Kamailio API side is already freed by * db_mysql_convert_result in case of error, but we also need * to free the mem from the mysql lib side */ - mysql_free_result(CON_RESULT(_h)); + mysql_free_result(RES_RESULT(*_r)); #if (MYSQL_VERSION_ID >= 40100) while( mysql_more_results(CON_CONNECTION(_h)) && mysql_next_result(CON_CONNECTION(_h)) > 0 ) { MYSQL_RES *res = mysql_store_result( CON_CONNECTION(_h) ); mysql_free_result(res); } #endif - CON_RESULT(_h) = 0; + RES_RESULT(*_r) = 0; return -4; } @@ -224,19 +224,21 @@ done: * \param _r result set that should be freed * \return zero on success, negative value on failure */ -int db_mysql_free_result(db1_con_t* _h, db1_res_t* _r) +int db_mysql_free_result(const db1_con_t* _h, db1_res_t* _r) { if ((!_h) || (!_r)) { LM_ERR("invalid parameter value\n"); return -1; } + mysql_free_result(RES_RESULT(_r)); + RES_RESULT(_r) = 0; + pkg_free(RES_PTR(_r)); + if (db_free_result(_r) < 0) { LM_ERR("unable to free result structure\n"); return -1; } - mysql_free_result(CON_RESULT(_h)); - CON_RESULT(_h) = 0; return 0; } @@ -288,21 +290,21 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows) /* exit if the fetch count is zero */ if (nrows == 0) { - db_free_result(*_r); + db_mysql_free_result(_h, *_r); *_r = 0; return 0; } if(*_r==0) { /* Allocate a new result structure */ - *_r = db_new_result(); + *_r = db_mysql_new_result(); if (*_r == 0) { LM_ERR("no memory left\n"); return -2; } - CON_RESULT(_h) = mysql_store_result(CON_CONNECTION(_h)); - if (!CON_RESULT(_h)) { + RES_RESULT(*_r) = mysql_store_result(CON_CONNECTION(_h)); + if (!RES_RESULT(*_r)) { if (mysql_field_count(CON_CONNECTION(_h)) == 0) { (*_r)->col.n = 0; (*_r)->n = 0; @@ -313,7 +315,7 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows) if (code == CR_SERVER_GONE_ERROR || code == CR_SERVER_LOST) { counter_inc(mysql_cnts_h.driver_err); } - db_free_result(*_r); + db_mysql_free_result(_h, *_r); *_r = 0; return -3; } @@ -323,7 +325,7 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows) return -4; } - RES_NUM_ROWS(*_r) = mysql_num_rows(CON_RESULT(_h)); + RES_NUM_ROWS(*_r) = mysql_num_rows(RES_RESULT(*_r)); if (!RES_NUM_ROWS(*_r)) { LM_DBG("no rows returned from the query\n"); RES_ROWS(*_r) = 0; @@ -362,8 +364,8 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows) } for(i = 0; i < rows; i++) { - CON_ROW(_h) = mysql_fetch_row(CON_RESULT(_h)); - if (!CON_ROW(_h)) { + RES_ROW(*_r) = mysql_fetch_row(RES_RESULT(*_r)); + if (!RES_ROW(*_r)) { LM_ERR("driver error: %s\n", mysql_error(CON_CONNECTION(_h))); RES_ROW_N(*_r) = i; db_free_rows(*_r); diff --git a/modules/db_mysql/km_dbase.h b/modules/db_mysql/km_dbase.h index 3a567f2..2acf517 100644 --- a/modules/db_mysql/km_dbase.h +++ b/modules/db_mysql/km_dbase.h @@ -57,7 +57,7 @@ void db_mysql_close(db1_con_t* _h); /*! \brief * Free all memory allocated by get_result */ -int db_mysql_free_result(db1_con_t* _h, db1_res_t* _r); +int db_mysql_free_result(const db1_con_t* _h, db1_res_t* _r); /*! \brief diff --git a/modules/db_mysql/km_my_con.c b/modules/db_mysql/km_my_con.c index ce20be2..646d74b 100644 --- a/modules/db_mysql/km_my_con.c +++ b/modules/db_mysql/km_my_con.c @@ -144,7 +144,6 @@ void db_mysql_free_connection(struct pool_con* con) _c = (struct my_con*) con; - if (_c->res) mysql_free_result(_c->res); if (_c->id) free_db_id(_c->id); if (_c->con) { mysql_close(_c->con); diff --git a/modules/db_mysql/km_my_con.h b/modules/db_mysql/km_my_con.h index 2624f34..3d71d00 100644 --- a/modules/db_mysql/km_my_con.h +++ b/modules/db_mysql/km_my_con.h @@ -44,9 +44,7 @@ struct my_con { unsigned int ref; /*!< Reference count */ struct pool_con* next; /*!< Next connection in the pool */ - MYSQL_RES* res; /*!< Actual result */ MYSQL* con; /*!< Connection representation */ - MYSQL_ROW row; /*!< Actual row in the result */ time_t timestamp; /*!< Timestamp of last query */ }; @@ -54,9 +52,7 @@ struct my_con { /* * Some convenience wrappers */ -#define CON_RESULT(db_con) (((struct my_con*)((db_con)->tail))->res) #define CON_CONNECTION(db_con) (((struct my_con*)((db_con)->tail))->con) -#define CON_ROW(db_con) (((struct my_con*)((db_con)->tail))->row) #define CON_TIMESTAMP(db_con) (((struct my_con*)((db_con)->tail))->timestamp) diff --git a/modules/db_mysql/km_res.c b/modules/db_mysql/km_res.c index 10d3ce2..965e6dc 100644 --- a/modules/db_mysql/km_res.c +++ b/modules/db_mysql/km_res.c @@ -73,7 +73,7 @@ int db_mysql_get_columns(const db1_con_t* _h, db1_res_t* _r) return -3; } - fields = mysql_fetch_fields(CON_RESULT(_h)); + fields = mysql_fetch_fields(RES_RESULT(_r)); for(col = 0; col < RES_COL_N(_r); col++) { RES_NAMES(_r)[col] = (str*)pkg_malloc(sizeof(str)); if (! RES_NAMES(_r)[col]) { @@ -164,7 +164,7 @@ static inline int db_mysql_convert_rows(const db1_con_t* _h, db1_res_t* _r) return -1; } - RES_ROW_N(_r) = mysql_num_rows(CON_RESULT(_h)); + RES_ROW_N(_r) = mysql_num_rows(RES_RESULT(_r)); if (!RES_ROW_N(_r)) { LM_DBG("no rows returned from the query\n"); RES_ROWS(_r) = 0; @@ -177,8 +177,8 @@ static inline int db_mysql_convert_rows(const db1_con_t* _h, db1_res_t* _r) } for(row = 0; row < RES_ROW_N(_r); row++) { - CON_ROW(_h) = mysql_fetch_row(CON_RESULT(_h)); - if (!CON_ROW(_h)) { + RES_ROW(_r) = mysql_fetch_row(RES_RESULT(_r)); + if (!RES_ROW(_r)) { LM_ERR("driver error: %s\n", mysql_error(CON_CONNECTION(_h))); RES_ROW_N(_r) = row; db_free_rows(_r); @@ -221,3 +221,22 @@ int db_mysql_convert_result(const db1_con_t* _h, db1_res_t* _r) return 0; } + +/*! + * \brief Allocate new result set with private structure + * \return db1_res_t object on success, NULL on failure + */ +db1_res_t* db_mysql_new_result(void) +{ + db1_res_t* obj; + + obj = db_new_result(); + if (!obj) + return NULL; + RES_PTR(obj) = pkg_malloc(sizeof(struct my_res)); + if (!RES_PTR(obj)) { + db_free_result(obj); + return NULL; + } + return obj; +} diff --git a/modules/db_mysql/km_res.h b/modules/db_mysql/km_res.h index 957a961..0b063d8 100644 --- a/modules/db_mysql/km_res.h +++ b/modules/db_mysql/km_res.h @@ -37,6 +37,18 @@ #include "../../lib/srdb1/db_con.h" +struct my_res { + MYSQL_RES* res; /*!< Actual result */ + MYSQL_ROW row; /*!< Actual row in the result */ +}; + +/* + * Some convenience wrappers + */ +#define RES_RESULT(db_res) (((struct my_res*)((db_res)->ptr))->res) +#define RES_ROW(db_res) (((struct my_res*)((db_res)->ptr))->row) + + /*! * \brief Fill the result structure with data from database * \param _h database connection @@ -54,4 +66,11 @@ int db_mysql_convert_result(const db1_con_t* _h, db1_res_t* _r); */ int db_mysql_get_columns(const db1_con_t* _h, db1_res_t* _r); + +/*! + * \brief Allocate new result set with private structure + * \return db1_res_t object on success, NULL on failure + */ +db1_res_t* db_mysql_new_result(void); + #endif diff --git a/modules/db_mysql/km_row.c b/modules/db_mysql/km_row.c index 42f2841..9f2c730 100644 --- a/modules/db_mysql/km_row.c +++ b/modules/db_mysql/km_row.c @@ -36,6 +36,7 @@ #include "km_my_con.h" #include "km_val.h" #include "km_row.h" +#include "km_res.h" /*! * \brief Convert a row from result into DB API representation @@ -59,11 +60,11 @@ int db_mysql_convert_row(const db1_con_t* _h, db1_res_t* _res, db_row_t* _r) return -2; } - lengths = mysql_fetch_lengths(CON_RESULT(_h)); + lengths = mysql_fetch_lengths(RES_RESULT(_res)); for(i = 0; i < RES_COL_N(_res); i++) { if (db_str2val(RES_TYPES(_res)[i], &(ROW_VALUES(_r)[i]), - ((MYSQL_ROW)CON_ROW(_h))[i], lengths[i], 0) < 0) { + ((MYSQL_ROW)RES_ROW(_res))[i], lengths[i], 0) < 0) { LM_ERR("failed to convert value\n"); LM_DBG("free row at %p\n", _r); db_free_row(_r);