[SR-Dev] git:janakj/postgres: - change behaviour of db_str2val, this now copy strings

Module: sip-router Branch: janakj/postgres Commit: e397c728639835065a3097491b7afc45ee44b1d9 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=e397c72… Author: Henning Westerholt &lt;henning.westerholt(a)1und1.de&gt; Committer: Henning Westerholt &lt;henning.westerholt(a)1und1.de&gt; Date: Mon Dec 15 15:07:12 2008 +0000 - change behaviour of db_str2val, this now copy strings - this change was necessary because in some circumstances the memory that is returned from the database driver is used by some other means too fast, causes crashed or wrong results later on, this bug is timing related - this means for mysql and unixodbc that the performance will be decreased a bit, postgres already copied the string, so here nothing change - add a new function to DB core API, db_val2str that, usable to convert numerical values to a string, and common things like NULL values conversion, parameter checks - convert mysql, postgres and unixodbc module to use this new function - convert postgres function to use the core db_str2val function, the copying is now done at a different place, cleanup the code somewhat - remove unnecessary assignment if a NULL value is encountered in postgres - TODO: fix NULL handling and double copying that is done now for unixodbc git-svn-id: https://openser.svn.sourceforge.net/svnroot/openser/trunk@5359 689a6050-402a-0410-94f2-e92a70836424 --- modules/db_postgres/km_res.c | 49 +----------- modules/db_postgres/km_val.c | 175 +++--------------------------------------- 2 files changed, 13 insertions(+), 211 deletions(-) diff --git a/modules/db_postgres/km_res.c b/modules/db_postgres/km_res.c index a234d1f..696327a 100644 --- a/modules/db_postgres/km_res.c +++ b/modules/db_postgres/km_res.c @@ -250,19 +250,9 @@ int db_postgres_convert_rows(const db_con_t* _h, db_res_t* _r) * steps expect. So we need to simulate this here unfortunally. */ if (PQgetisnull(CON_RESULT(_h), row, col) == 0) { - len = strlen(s); - row_buf[col] = pkg_malloc(len+1); - if (!row_buf[col]) { - LM_ERR("no private memory left\n"); - return -1; - } - memset(row_buf[col], 0, len+1); - LM_DBG("allocated %d bytes for row_buf[%d] at %p\n", len, col, row_buf[col]); - strncpy(row_buf[col], s, len); - LM_DBG("[%d][%d] Column[%.*s]=[%s]\n", + row_buf[col] = s; + LM_ERR("[%d][%d] Column[%.*s]=[%s]\n", row, col, RES_NAMES(_r)[col]->len, RES_NAMES(_r)[col]->s, row_buf[col]); - } else { - s = NULL; } } @@ -277,44 +267,9 @@ int db_postgres_convert_rows(const db_con_t* _h, db_res_t* _r) LM_DBG("freeing row buffer at %p\n", row_buf); pkg_free(row_buf); return -4; - } - /* - * pkg_free() must be done for the above allocations now that the row - * has been converted. During pg_convert_row (and subsequent pg_str2val) - * processing, data types that don't need to be converted (namely STRINGS - * and STR) have their addresses saved. These data types should not have - * their pkg_malloc() allocations freed here because they are still - * needed. However, some data types (ex: INT, DOUBLE) should have their - * pkg_malloc() allocations freed because during the conversion process, - * their converted values are saved in the union portion of the db_val_t - * structure. BLOB will be copied during PQunescape in str2val, thus it - * has to be freed here AND in pg_free_row(). - * - * Warning: when the converted row is no longer needed, the data types - * whose addresses were saved in the db_val_t structure must be freed - * or a memory leak will happen. This processing should happen in the - * pg_free_row() subroutine. The caller of this routine should ensure - * that pg_free_rows(), pg_free_row() or pg_free_result() is eventually - * called. - */ - for (col = 0; col < RES_COL_N(_r); col++) { - switch (RES_TYPES(_r)[col]) { - case DB_STRING: - case DB_STR: - break; - default: - LM_DBG("freeing row_buf[%d] at %p\n", col, row_buf[col]); - /* because it can contain NULL */ - if(row_buf[col]) pkg_free(row_buf[col]); - } /* * The following housekeeping may not be technically required, but it * is a good practice to NULL pointer fields that are no longer valid. - * Note that DB_STRING fields have not been pkg_free(). NULLing DB_STRING - * fields would normally not be good to do because a memory leak would - * occur. However, the pg_convert_row() routine has saved the DB_STRING - * pointer in the db_val_t structure. The db_val_t structure will - * eventually be used to pkg_free() the DB_STRING storage. */ row_buf[col] = (char *)NULL; } diff --git a/modules/db_postgres/km_val.c b/modules/db_postgres/km_val.c index 9b7d12b..c83a1b2 100644 --- a/modules/db_postgres/km_val.c +++ b/modules/db_postgres/km_val.c @@ -42,14 +42,11 @@ #include "../../mem/mem.h" #include "val.h" -#include <string.h> -#include <time.h> - /*! - * \brief Convert a str to a db value, does not copy strings + * \brief Convert a str to a db value, copy strings * - * Convert a str to a db value, does not copy strings. + * Convert a str to a db value, copy strings. * The postgresql module uses a custom escape function for BLOBs. * If the _s is linked in the db_val result, it will be returned zero * \param _t destination value type @@ -60,105 +57,10 @@ */ int db_postgres_str2val(const db_type_t _t, db_val_t* _v, const char* _s, const int _l) { - static str dummy_string = {"", 0}; - char *tmp_s; - - if (!_v) { - LM_ERR("invalid parameter value\n"); - } - - /* - * We implemented the mysql behaviour in the db_postgres_convert_rows function, - * so a NULL string is a NULL value, otherwise its an empty value - */ - if (!_s) { - memset(_v, 0, sizeof(db_val_t)); - /* Initialize the string pointers to a dummy empty - * string so that we do not crash when the NULL flag - * is set but the module does not check it properly - */ - VAL_STRING(_v) = dummy_string.s; - VAL_STR(_v) = dummy_string; - VAL_BLOB(_v) = dummy_string; - VAL_TYPE(_v) = _t; - VAL_NULL(_v) = 1; - return 0; - } - VAL_NULL(_v) = 0; - - switch(_t) { - case DB_INT: - LM_DBG("converting INT [%s]\n", _s); - if (db_str2int(_s, &VAL_INT(_v)) < 0) { - LM_ERR("failed to convert INT value from string\n"); - return -2; - } else { - VAL_TYPE(_v) = DB_INT; - return 0; - } - break; - - case DB_BIGINT: - LM_DBG("converting BIGINT [%s]\n", _s); - if (db_str2longlong(_s, &VAL_BIGINT(_v)) < 0) { - LM_ERR("failed to convert BIGINT value from string\n"); - return -3; - } else { - VAL_TYPE(_v) = DB_BIGINT; - return 0; - } - break; - - case DB_BITMAP: - LM_DBG("converting BITMAP [%s]\n", _s); - if (db_str2int(_s, &VAL_INT(_v)) < 0) { - LM_ERR("failed to convert BITMAP value from string\n"); - return -4; - } else { - VAL_TYPE(_v) = DB_BITMAP; - return 0; - } - break; - - case DB_DOUBLE: - LM_DBG("converting DOUBLE [%s]\n", _s); - if (db_str2double(_s, &VAL_DOUBLE(_v)) < 0) { - LM_ERR("failed to convert DOUBLE value from string\n"); - return -5; - } else { - VAL_TYPE(_v) = DB_DOUBLE; - return 0; - } - break; - - case DB_STRING: - LM_DBG("converting STRING [%s]\n", _s); - VAL_STRING(_v) = _s; - VAL_TYPE(_v) = DB_STRING; - VAL_FREE(_v) = 1; - return 0; - - case DB_STR: - LM_DBG("converting STR [%.*s]\n", _l, _s); - VAL_STR(_v).s = (char*)_s; - VAL_STR(_v).len = _l; - VAL_TYPE(_v) = DB_STR; - VAL_FREE(_v) = 1; - _s = 0; - return 0; - - case DB_DATETIME: - LM_DBG("converting DATETIME [%s]\n", _s); - if (db_str2time(_s, &VAL_TIME(_v)) < 0) { - LM_ERR("failed to convert datetime\n"); - return -6; - } else { - VAL_TYPE(_v) = DB_DATETIME; - return 0; - } - break; - - case DB_BLOB: + if ( (_t != DB_BLOB && _v != NULL) || _v == NULL) { + return db_str2val(_t, _v, _s, _l); + } else { + char * tmp_s = NULL; LM_DBG("converting BLOB [%.*s]\n", _l, _s); /* * The string is stored in new allocated memory, which we could @@ -184,8 +86,8 @@ int db_postgres_str2val(const db_type_t _t, db_val_t* _v, const char* _s, const LM_DBG("got blob len %d\n", _l); return 0; + } - return -9; } @@ -201,63 +103,17 @@ int db_postgres_str2val(const db_type_t _t, db_val_t* _v, const char* _s, const */ int db_postgres_val2str(const db_con_t* _con, const db_val_t* _v, char* _s, int* _len) { - int l, ret; + int l, ret, tmp; int pgret; char *tmp_s; size_t tmp_len; char* old_s; - if ((!_v) || (!_s) || (!_len) || (!*_len)) { - LM_ERR("invalid parameter value\n"); - return -1; - } + tmp = db_val2str(_con, _v, _s, _len); + if (tmp < 1) + return tmp; - if (VAL_NULL(_v)) { - if (*_len < sizeof("NULL")) { - LM_ERR("buffer too small\n"); - return -1; - } - *_len = snprintf(_s, *_len, "NULL"); - return 0; - } - switch(VAL_TYPE(_v)) { - case DB_INT: - if (db_int2str(VAL_INT(_v), _s, _len) < 0) { - LM_ERR("failed to convert string to int\n"); - return -2; - } else { - return 0; - } - break; - - case DB_BIGINT: - if (db_longlong2str(VAL_BIGINT(_v), _s, _len) < 0) { - LM_ERR("failed to convert string to big int\n"); - return -3; - } else { - return 0; - } - break; - - case DB_BITMAP: - if (db_int2str(VAL_BITMAP(_v), _s, _len) < 0) { - LM_ERR("failed to convert string to int\n"); - return -4; - } else { - return 0; - } - break; - - case DB_DOUBLE: - if (db_double2str(VAL_DOUBLE(_v), _s, _len) < 0) { - LM_ERR("failed to convert string to double\n"); - return -5; - } else { - return 0; - } - break; - case DB_STRING: l = strlen(VAL_STRING(_v)); if (*_len < (l * 2 + 3)) { @@ -307,15 +163,6 @@ int db_postgres_val2str(const db_con_t* _con, const db_val_t* _v, char* _s, int* } break; - case DB_DATETIME: - if (db_time2str(VAL_TIME(_v), _s, _len) < 0) { - LM_ERR("failed to convert string to time_t\n"); - return -8; - } else { - return 0; - } - break; - case DB_BLOB: l = VAL_BLOB(_v).len; /* this estimation is not always correct, thus we need to check later again */