further improvements of the database API:

- unify postgres, unixodbc and mysql code, error and debug messages
- move the column allocating logic into the DB core API
- change the all three modules to use this function
- change postgres module to use the memory memory management methods of the core
- add a flag to the db_val_t type that indicates if the (string) memory must be freed
- don't try to free the static NULL values in db_free_row
- change core row freeing method to evaluate the memory status flag
- change the postgres module to set this flag
- add some memsets to the mysql and unixodbc module for more safety and also
  ensure the correct working of the free flag
- fix database TYPE flags in mysql module, they will be removed for 5.2 and are
  also obselete since 3.X times
- change column names allocation logic in postgres to match the unixodbc and mysql
  behaviour, don't copy the names
- some smaller documentation improvements and cleanups
- tested only with testsuite so far

works that still needs to be done:
- modify the db_berkeley, dbtext and flatstore module to use the core API
- modify the postgres, db_berkeley driver that they don't copy the values
  returned from the driver


git-svn-id: https://openser.svn.sourceforge.net/svnroot/openser/trunk@3637 689a6050-402a-0410-94f2-e92a70836424

lib/srdb1/db.h

@@ -44,10 +44,10 @@
 #include "db_op.h"
 #include "db_val.h"
 #include "db_con.h"
-#include "db_row.h"
 #include "db_res.h"
 #include "db_cap.h"
 #include "db_con.h"
+#include "db_row.h"
 
 
 /**

lib/srdb1/db_res.c

@@ -51,13 +51,12 @@ inline int db_free_rows(db_res_t* _r)
 	LM_DBG("freeing %d rows\n", RES_ROW_N(_r));
 
 	for(i = 0; i < RES_ROW_N(_r); i++) {
-		LM_DBG("row[%d]=%p\n", i, &(RES_ROWS(_r)[i]));
 		db_free_row(&(RES_ROWS(_r)[i]));
 	}
 	RES_ROW_N(_r) = 0;
 
 	if (RES_ROWS(_r)) {
-		LM_DBG("%p=pkg_free() RES_ROWS\n", RES_ROWS(_r));
+		LM_DBG("freeing rows at %p\n", RES_ROWS(_r));
 		pkg_free(RES_ROWS(_r));
 		RES_ROWS(_r) = NULL;
 	}
@@ -70,17 +69,33 @@ inline int db_free_rows(db_res_t* _r)
  */
 inline int db_free_columns(db_res_t* _r)
 {
+	int col;
+
 	if (!_r) {
 		LM_ERR("invalid parameter value\n");
 		return -1;
 	}
-
-	if (RES_NAMES(_r)) pkg_free(RES_NAMES(_r));
-	if (RES_TYPES(_r)) pkg_free(RES_TYPES(_r));
+	LM_DBG("freeing %d columns\n", RES_COL_N(_r));
+	/* free memory previously allocated to save column names */
+	for(col = 0; col < RES_COL_N(_r); col++) {
+		LM_DBG("freeing RES_NAMES[%d] at %p\n", col, RES_NAMES(_r)[col]);
+		pkg_free((str *)RES_NAMES(_r)[col]);
+		RES_NAMES(_r)[col] = NULL;
+	}
+	/* free names and types */
+	if (RES_NAMES(_r)) {
+		LM_DBG("freeing result names at %p\n", RES_NAMES(_r));
+		pkg_free(RES_NAMES(_r));
+		RES_NAMES(_r) = NULL;
+	}
+	if (RES_TYPES(_r)) {
+		LM_DBG("freeing result types at %p\n", RES_TYPES(_r));
+		pkg_free(RES_TYPES(_r));
+		RES_TYPES(_r) = NULL;
+	}
 	return 0;
 }
 
-
 /*
  * Create a new result structure and initialize it
  */
@@ -88,11 +103,11 @@ inline db_res_t* db_new_result(void)
 {
 	db_res_t* r = NULL;
 	r = (db_res_t*)pkg_malloc(sizeof(db_res_t));
-	//LM_DBG("%p=pkg_malloc(%lu) _res\n", _r, (unsigned long)sizeof(db_res_t));
 	if (!r) {
 		LM_ERR("no private memory left\n");
 		return 0;
 	}
+	LM_DBG("allocate %d bytes for result set at %p\n", sizeof(db_res_t), r);
 	memset(r, 0, sizeof(db_res_t));
 	return r;
 }
@@ -110,6 +125,34 @@ inline int db_free_result(db_res_t* _r)
 
 	db_free_columns(_r);
 	db_free_rows(_r);
+	LM_DBG("freeing result set at %p\n", _r);
 	pkg_free(_r);
+	_r = NULL;
+	return 0;
+}
+
+/*
+ * Allocate storage for column names and type in existing
+ * result structure.
+ */
+inline int db_allocate_columns(db_res_t* _r, const unsigned int cols)
+{
+	RES_NAMES(_r) = (db_key_t*)pkg_malloc(sizeof(db_key_t) * cols);
+	if (!RES_NAMES(_r)) {
+		LM_ERR("no private memory left\n");
+		return -1;
+	}
+	LM_DBG("allocate %d bytes for result names at %p\n", sizeof(db_key_t) * cols,
+		RES_NAMES(_r));
+
+	RES_TYPES(_r) = (db_type_t*)pkg_malloc(sizeof(db_type_t) * cols);
+	if (!RES_TYPES(_r)) {
+		LM_ERR("no private memory left\n");
+		pkg_free(RES_NAMES(_r));
+		return -1;
+	}
+	LM_DBG("allocate %d bytes for result types at %p\n", sizeof(db_type_t) * cols,
+		RES_TYPES(_r));
+
 	return 0;
 }

lib/srdb1/db_res.h

@@ -34,21 +34,21 @@
 #define DB_RES_H
 
 
-#include "db_row.h"
 #include "db_key.h"
 #include "db_val.h"
 
+struct db_row;
 
 /**
  * This type represents a result returned by db_query function (see below). The 
  * result can consist of zero or more rows (see db_row_t description).
- * 
+ *
  * Note: A variable of type db_res_t returned by db_query function uses dynamicaly
  * allocated memory, don't forget to call db_free_result if you don't need the
  * variable anymore. You will encounter memory leaks if you fail to do this!
  *
  * In addition to zero or more rows, each db_res_t object contains also an array
- * of db_key_t objects. The objects represent keys (names of columns).
+ * of db_key_t objects. The objects represent keys (names of columns). *
  */
 typedef struct db_res {
 	struct {
@@ -80,7 +80,7 @@ typedef struct db_res {
 
 
 /**
- * Release memory used by rows in a result structure
+ * Release memory used by rows in a result structure.
  * \param _r the result that should be released
  * \return zero on success, negative on errors
  */
@@ -88,15 +88,17 @@ inline int db_free_rows(db_res_t* _r);
 
 
 /**
- * Release memory used by columns
+ * Release memory used by columns. This methods assumes that the string values
+ * holding the column names are in memory allocated from the database driver,
+ * and thus must be not freed here.
  * \param _r the result that should be released
  * \return zero on success, negative on errors
  */
-int db_free_columns(db_res_t* _r);
+inline int db_free_columns(db_res_t* _r);
 
 
 /**
- * Create a new result structure and initialize it
+ * Create a new result structure and initialize it.
  * \return a pointer to the new result on success, NULL on errors
  */
 inline db_res_t* db_new_result(void);
@@ -105,6 +107,16 @@ inline db_res_t* db_new_result(void);
  * Release memory used by a result structure.
  * \return zero on success, negative on errors
  */
-int db_free_result(db_res_t* _r);
+inline int db_free_result(db_res_t* _r);
+
+/**
+ * Allocate storage for column names and type in existing result structure.
+ * If no more memory is available for the allocation of the types then the
+ * already allocated memory for the names is freed.
+ * \param _r filled result set
+ * \param cols number of columns
+ * \return zero on success, negative on errors
+ */
+inline int db_allocate_columns(db_res_t* _r, const unsigned int cols);
 
 #endif /* DB_RES_H */

lib/srdb1/db_row.c

@@ -31,6 +31,7 @@
 
 #include "db_row.h"
 
+#include <string.h>
 #include "../dprint.h"
 #include "../mem/mem.h"
 
@@ -39,11 +40,58 @@
  */
 inline int db_free_row(db_row_t* _r)
 {
+	int	col;
+	db_val_t* _val;
+
 	if (!_r) {
 		LM_ERR("invalid parameter value\n");
 		return -1;
 	}
 
-	if (ROW_VALUES(_r)) pkg_free(ROW_VALUES(_r));
+	/*
+	 * Loop thru each columm, then check to determine if the storage pointed to
+	 * by db_val_t structure must be freed. This is required for all data types
+	 * which use a pointer to a buffer like DB_STRING, DB_STR and DB_BLOB and
+	 * the database module copied them during the assignment.
+	 * If this is not done, a memory leak will happen.
+	 * Don't try to free the static dummy string (as indicated from the NULL value),
+	 * as this is not valid.
+	 */
+	for (col = 0; col < ROW_N(_r); col++) {
+		_val = &(ROW_VALUES(_r)[col]);
+		switch (VAL_TYPE(_val)) {
+			case DB_STRING:
+				if ( (!VAL_NULL(_val)) && VAL_FREE(_val)) {
+					LM_DBG("free VAL_STRING[%d] '%s' at %p\n", col, (char *)VAL_STRING(_val),
+							(char *)VAL_STRING(_val));
+					pkg_free((char *)VAL_STRING(_val));
+					VAL_STRING(_val) = NULL;
+				}
+				break;
+			case DB_STR:
+				if ( (!VAL_NULL(_val)) && VAL_FREE(_val)) {
+					LM_DBG("free VAL_STR[%d] '%.*s' at %p\n", col, VAL_STR(_val).len,
+							VAL_STR(_val).s, VAL_STR(_val).s);
+					pkg_free(VAL_STR(_val).s);
+					VAL_STR(_val).s = NULL;
+				}
+				break;
+			case DB_BLOB:
+				if ( (!VAL_NULL(_val)) && VAL_FREE(_val)) {
+					LM_DBG("free VAL_BLOB[%d] at %p\n", col, VAL_BLOB(_val).s);
+					pkg_free(VAL_BLOB(_val).s);
+					VAL_BLOB(_val).s = NULL;
+				}
+				break;
+			default:
+				break;
+		}
+	}
+
+	if (ROW_VALUES(_r)) {
+		LM_DBG("freeing row values at %p\n", ROW_VALUES(_r));
+		pkg_free(ROW_VALUES(_r));
+		ROW_VALUES(_r) = NULL;
+	}
 	return 0;
 }

lib/srdb1/db_row.h

@@ -49,14 +49,17 @@ typedef struct db_row {
 
 /** Return the columns in the row */
 #define ROW_VALUES(rw) ((rw)->values)
-/** Return the number of rows */
+/** Return the number of colums */
 #define ROW_N(rw)      ((rw)->n)
 
 /**
- * Release memory used by row
- * \param _r the row that should be released
+ * Release memory used by a row. This method only frees values that are inside
+ * the row if the free flag of the specific value is set. Otherwise this
+ * storage must be released when the database specific result free function is
+ * called. Only string based values are freed if wanted, null values are skipped.
+ * \param _r row that should be released
  * \return zero on success, negative on error
  */
-int db_free_row(db_row_t* _r);
+inline int db_free_row(db_row_t* _r);
 
 #endif /* DB_ROW_H */

lib/srdb1/db_val.h

@@ -61,10 +61,19 @@ typedef enum {
  * recognized and converted by the database API. These datatypes are automaticaly
  * recognized, converted from internal database representation and stored in the
  * variable of corresponding type.
+ *
+ * Module that want to use this values needs to copy them to another memory
+ * location, because after the call to free_result there are not more available.
+ *
+ * If the structure holds a pointer to a string value that needs to be freed
+ * because the module allocated new memory for it then the free flag must
+ * be set to a non-zero value. A free flag of zero means that the string
+ * data must be freed internally by the database driver.
  */
 typedef struct {
-	db_type_t type; /** Type of the value                              */
-	int nul;        /** Means that the column in database has no value */
+	db_type_t type; /**< Type of the value                              */
+	int nul;		/**< Means that the column in database has no value */
+	int free;		/**< Means that the value should be freed */
 	/** Column value structure that holds the actual data in a union.  */
 	union {
 		int           int_val;    /**< integer value              */
@@ -97,6 +106,14 @@ typedef struct {
 #define VAL_NULL(dv)   ((dv)->nul)
 
 
+/**
+ * Use this macro if you need to set/ get the free flag. A non-zero flag means that
+ * the corresponding cell in the database contains data that must be freed from the
+ * DB API.
+ */
+#define VAL_FREE(dv)   ((dv)->free)
+
+
 /**
  * Use this macro if you need to access the integer value in the db_val_t structure.
  */