[asterisk-commits] jrose: branch 1.8 r406360 - /branches/1.8/res/res_config_pgsql.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Jan 24 14:57:52 CST 2014


Author: jrose
Date: Fri Jan 24 14:57:50 2014
New Revision: 406360

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=406360
Log:
res_config_pgsql: Fix a memory leak and use RAII_VAR for cleanup when practical

Review: https://reviewboard.asterisk.org/r/3141/

Modified:
    branches/1.8/res/res_config_pgsql.c

Modified: branches/1.8/res/res_config_pgsql.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_config_pgsql.c?view=diff&rev=406360&r1=406359&r2=406360
==============================================================================
--- branches/1.8/res/res_config_pgsql.c (original)
+++ branches/1.8/res/res_config_pgsql.c Fri Jan 24 14:57:50 2014
@@ -132,7 +132,7 @@
 	struct tables *table;
 	struct ast_str *sql = ast_str_thread_get(&findtable_buf, 330);
 	char *pgerror;
-	PGresult *result;
+	RAII_VAR(PGresult *, result, NULL, PQclear);
 	char *fname, *ftype, *flen, *fnotnull, *fdef;
 	int i, rows;
 
@@ -222,7 +222,6 @@
 	if (PQresultStatus(result) != PGRES_TUPLES_OK) {
 		pgerror = PQresultErrorMessage(result);
 		ast_log(LOG_ERROR, "Failed to query database columns: %s\n", pgerror);
-		PQclear(result);
 		AST_LIST_UNLOCK(&psql_tables);
 		ast_mutex_unlock(&pgsql_lock);
 		return NULL;
@@ -230,7 +229,6 @@
 
 	if (!(table = ast_calloc(1, sizeof(*table) + strlen(orig_tablename) + 1))) {
 		ast_log(LOG_ERROR, "Unable to allocate memory for new table structure\n");
-		PQclear(result);
 		AST_LIST_UNLOCK(&psql_tables);
 		ast_mutex_unlock(&pgsql_lock);
 		return NULL;
@@ -250,7 +248,6 @@
 
 		if (!(column = ast_calloc(1, sizeof(*column) + strlen(fname) + strlen(ftype) + 2))) {
 			ast_log(LOG_ERROR, "Unable to allocate column element for %s, %s\n", orig_tablename, fname);
-			PQclear(result);
 			destroy_table(table);
 			AST_LIST_UNLOCK(&psql_tables);
 			ast_mutex_unlock(&pgsql_lock);
@@ -281,7 +278,6 @@
 		}
 		AST_LIST_INSERT_TAIL(&table->columns, column, list);
 	}
-	PQclear(result);
 
 	AST_LIST_INSERT_TAIL(&psql_tables, table, list);
 	ast_rwlock_rdlock(&table->lock);
@@ -307,7 +303,7 @@
 
 static struct ast_variable *realtime_pgsql(const char *database, const char *tablename, va_list ap)
 {
-	PGresult *result = NULL;
+	RAII_VAR(PGresult *, result, NULL, PQclear);
 	int num_rows = 0, pgresult;
 	struct ast_str *sql = ast_str_thread_get(&sql_buf, 100);
 	struct ast_str *escapebuf = ast_str_thread_get(&escapebuf_buf, 100);
@@ -380,7 +376,6 @@
 				"PostgreSQL RealTime: Failed to query '%s@%s'. Check debug for more info.\n", tablename, database);
 		ast_debug(1, "PostgreSQL RealTime: Query: %s\n", ast_str_buffer(sql));
 		ast_debug(1, "PostgreSQL RealTime: Query Failed because: %s\n", PQerrorMessage(pgsqlConn));
-		PQclear(result);
 		ast_mutex_unlock(&pgsql_lock);
 		return NULL;
 	} else {
@@ -409,7 +404,6 @@
 		ast_debug(1, "PostgreSQL RealTime: Found %d rows.\n", num_rows);
 
 		if (!(fieldnames = ast_calloc(1, numFields * sizeof(char *)))) {
-			PQclear(result);
 			ast_mutex_unlock(&pgsql_lock);
 			return NULL;
 		}
@@ -438,7 +432,6 @@
 		ast_debug(1, "Postgresql RealTime: Could not find any rows in table %s@%s.\n", tablename, database);
 	}
 
-	PQclear(result);
 	ast_mutex_unlock(&pgsql_lock);
 
 	return var;
@@ -446,7 +439,7 @@
 
 static struct ast_config *realtime_multi_pgsql(const char *database, const char *table, va_list ap)
 {
-	PGresult *result = NULL;
+	RAII_VAR(PGresult *, result, NULL, PQclear);
 	int num_rows = 0, pgresult;
 	struct ast_str *sql = ast_str_thread_get(&sql_buf, 100);
 	struct ast_str *escapebuf = ast_str_thread_get(&escapebuf_buf, 100);
@@ -556,7 +549,6 @@
 			ast_debug(1, "PostgreSQL RealTime: Query: %s\n", ast_str_buffer(sql));
 			ast_debug(1, "PostgreSQL RealTime: Query Failed because: %s (%s)\n",
 						PQresultErrorMessage(result), PQresStatus(result_status));
-			PQclear(result);
 			ast_mutex_unlock(&pgsql_lock);
 			ast_config_destroy(cfg);
 			return NULL;
@@ -574,7 +566,6 @@
 		ast_debug(1, "PostgreSQL RealTime: Found %d rows.\n", num_rows);
 
 		if (!(fieldnames = ast_calloc(1, numFields * sizeof(char *)))) {
-			PQclear(result);
 			ast_mutex_unlock(&pgsql_lock);
 			ast_config_destroy(cfg);
 			return NULL;
@@ -606,7 +597,6 @@
 		ast_debug(1, "PostgreSQL RealTime: Could not find any rows in table %s.\n", table);
 	}
 
-	PQclear(result);
 	ast_mutex_unlock(&pgsql_lock);
 
 	return cfg;
@@ -615,7 +605,7 @@
 static int update_pgsql(const char *database, const char *tablename, const char *keyfield,
 						const char *lookup, va_list ap)
 {
-	PGresult *result = NULL;
+	RAII_VAR(PGresult *, result, NULL, PQclear);
 	int numrows = 0, pgresult;
 	const char *newparam, *newval;
 	struct ast_str *sql = ast_str_thread_get(&sql_buf, 100);
@@ -707,144 +697,6 @@
 	ast_debug(1, "PostgreSQL RealTime: Update SQL: %s\n", ast_str_buffer(sql));
 
 	/* We now have our complete statement; Lets connect to the server and execute it. */
-	ast_mutex_lock(&pgsql_lock);
-	if (!pgsql_reconnect(database)) {
-		ast_mutex_unlock(&pgsql_lock);
-		return -1;
-	}
-
-	if (!(result = PQexec(pgsqlConn, ast_str_buffer(sql)))) {
-		ast_log(LOG_WARNING,
-				"PostgreSQL RealTime: Failed to query database. Check debug for more info.\n");
-		ast_debug(1, "PostgreSQL RealTime: Query: %s\n", ast_str_buffer(sql));
-		ast_debug(1, "PostgreSQL RealTime: Query Failed because: %s\n", PQerrorMessage(pgsqlConn));
-		ast_mutex_unlock(&pgsql_lock);
-		return -1;
-	} else {
-		ExecStatusType result_status = PQresultStatus(result);
-		if (result_status != PGRES_COMMAND_OK
-			&& result_status != PGRES_TUPLES_OK
-			&& result_status != PGRES_NONFATAL_ERROR) {
-			ast_log(LOG_WARNING,
-					"PostgreSQL RealTime: Failed to query database. Check debug for more info.\n");
-			ast_debug(1, "PostgreSQL RealTime: Query: %s\n", ast_str_buffer(sql));
-			ast_debug(1, "PostgreSQL RealTime: Query Failed because: %s (%s)\n",
-						PQresultErrorMessage(result), PQresStatus(result_status));
-			PQclear(result);
-			ast_mutex_unlock(&pgsql_lock);
-			return -1;
-		}
-	}
-
-	numrows = atoi(PQcmdTuples(result));
-	ast_mutex_unlock(&pgsql_lock);
-
-	ast_debug(1, "PostgreSQL RealTime: Updated %d rows on table: %s\n", numrows, tablename);
-
-	/* From http://dev.pgsql.com/doc/pgsql/en/pgsql-affected-rows.html
-	 * An integer greater than zero indicates the number of rows affected
-	 * Zero indicates that no records were updated
-	 * -1 indicates that the query returned an error (although, if the query failed, it should have been caught above.)
-	 */
-
-	if (numrows >= 0)
-		return (int) numrows;
-
-	return -1;
-}
-
-static int update2_pgsql(const char *database, const char *tablename, va_list ap)
-{
-	PGresult *result = NULL;
-	int numrows = 0, pgresult, first = 1;
-	struct ast_str *escapebuf = ast_str_thread_get(&escapebuf_buf, 16);
-	const char *newparam, *newval;
-	struct ast_str *sql = ast_str_thread_get(&sql_buf, 100);
-	struct ast_str *where = ast_str_thread_get(&where_buf, 100);
-	struct tables *table;
-
-	/*
-	 * Ignore database from the extconfig.conf since it was
-	 * configured by res_pgsql.conf.
-	 */
-	database = dbname;
-
-	if (!tablename) {
-		ast_log(LOG_WARNING, "PostgreSQL RealTime: No table specified.\n");
-		return -1;
-	}
-
-	if (!escapebuf || !sql || !where) {
-		/* Memory error, already handled */
-		return -1;
-	}
-
-	if (!(table = find_table(database, tablename))) {
-		ast_log(LOG_ERROR, "Table '%s' does not exist!!\n", tablename);
-		return -1;
-	}
-
-	ast_str_set(&sql, 0, "UPDATE %s SET ", tablename);
-	ast_str_set(&where, 0, "WHERE");
-
-	while ((newparam = va_arg(ap, const char *))) {
-		if (!find_column(table, newparam)) {
-			ast_log(LOG_ERROR, "Attempted to update based on criteria column '%s' (%s@%s), but that column does not exist!\n", newparam, tablename, database);
-			release_table(table);
-			return -1;
-		}
-
-		newval = va_arg(ap, const char *);
-		ESCAPE_STRING(escapebuf, newval);
-		if (pgresult) {
-			ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
-			release_table(table);
-			ast_free(sql);
-			return -1;
-		}
-		ast_str_append(&where, 0, "%s %s='%s'", first ? "" : " AND", newparam, ast_str_buffer(escapebuf));
-		first = 0;
-	}
-
-	if (first) {
-		ast_log(LOG_WARNING,
-				"PostgreSQL RealTime: Realtime update requires at least 1 parameter and 1 value to search on.\n");
-		if (pgsqlConn) {
-			PQfinish(pgsqlConn);
-			pgsqlConn = NULL;
-		}
-		release_table(table);
-		return -1;
-	}
-
-	/* Now retrieve the columns to update */
-	first = 1;
-	while ((newparam = va_arg(ap, const char *))) {
-		newval = va_arg(ap, const char *);
-
-		/* If the column is not within the table, then skip it */
-		if (!find_column(table, newparam)) {
-			ast_log(LOG_NOTICE, "Attempted to update column '%s' in table '%s@%s', but column does not exist!\n", newparam, tablename, database);
-			continue;
-		}
-
-		ESCAPE_STRING(escapebuf, newval);
-		if (pgresult) {
-			ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
-			release_table(table);
-			ast_free(sql);
-			return -1;
-		}
-
-		ast_str_append(&sql, 0, "%s %s='%s'", first ? "" : ",", newparam, ast_str_buffer(escapebuf));
-	}
-	release_table(table);
-
-	ast_str_append(&sql, 0, " %s", ast_str_buffer(where));
-
-	ast_debug(1, "PostgreSQL RealTime: Update SQL: %s\n", ast_str_buffer(sql));
-
-	/* We now have our complete statement; connect to the server and execute it. */
 	ast_mutex_lock(&pgsql_lock);
 	if (!pgsql_reconnect(database)) {
 		ast_mutex_unlock(&pgsql_lock);
@@ -884,6 +736,143 @@
 	 * -1 indicates that the query returned an error (although, if the query failed, it should have been caught above.)
 	 */
 
+	if (numrows >= 0)
+		return (int) numrows;
+
+	return -1;
+}
+
+static int update2_pgsql(const char *database, const char *tablename, va_list ap)
+{
+	RAII_VAR(PGresult *, result, NULL, PQclear);
+	int numrows = 0, pgresult, first = 1;
+	struct ast_str *escapebuf = ast_str_thread_get(&escapebuf_buf, 16);
+	const char *newparam, *newval;
+	struct ast_str *sql = ast_str_thread_get(&sql_buf, 100);
+	struct ast_str *where = ast_str_thread_get(&where_buf, 100);
+	struct tables *table;
+
+	/*
+	 * Ignore database from the extconfig.conf since it was
+	 * configured by res_pgsql.conf.
+	 */
+	database = dbname;
+
+	if (!tablename) {
+		ast_log(LOG_WARNING, "PostgreSQL RealTime: No table specified.\n");
+		return -1;
+	}
+
+	if (!escapebuf || !sql || !where) {
+		/* Memory error, already handled */
+		return -1;
+	}
+
+	if (!(table = find_table(database, tablename))) {
+		ast_log(LOG_ERROR, "Table '%s' does not exist!!\n", tablename);
+		return -1;
+	}
+
+	ast_str_set(&sql, 0, "UPDATE %s SET ", tablename);
+	ast_str_set(&where, 0, "WHERE");
+
+	while ((newparam = va_arg(ap, const char *))) {
+		if (!find_column(table, newparam)) {
+			ast_log(LOG_ERROR, "Attempted to update based on criteria column '%s' (%s@%s), but that column does not exist!\n", newparam, tablename, database);
+			release_table(table);
+			return -1;
+		}
+
+		newval = va_arg(ap, const char *);
+		ESCAPE_STRING(escapebuf, newval);
+		if (pgresult) {
+			ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
+			release_table(table);
+			ast_free(sql);
+			return -1;
+		}
+		ast_str_append(&where, 0, "%s %s='%s'", first ? "" : " AND", newparam, ast_str_buffer(escapebuf));
+		first = 0;
+	}
+
+	if (first) {
+		ast_log(LOG_WARNING,
+				"PostgreSQL RealTime: Realtime update requires at least 1 parameter and 1 value to search on.\n");
+		if (pgsqlConn) {
+			PQfinish(pgsqlConn);
+			pgsqlConn = NULL;
+		}
+		release_table(table);
+		return -1;
+	}
+
+	/* Now retrieve the columns to update */
+	first = 1;
+	while ((newparam = va_arg(ap, const char *))) {
+		newval = va_arg(ap, const char *);
+
+		/* If the column is not within the table, then skip it */
+		if (!find_column(table, newparam)) {
+			ast_log(LOG_NOTICE, "Attempted to update column '%s' in table '%s@%s', but column does not exist!\n", newparam, tablename, database);
+			continue;
+		}
+
+		ESCAPE_STRING(escapebuf, newval);
+		if (pgresult) {
+			ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
+			release_table(table);
+			ast_free(sql);
+			return -1;
+		}
+
+		ast_str_append(&sql, 0, "%s %s='%s'", first ? "" : ",", newparam, ast_str_buffer(escapebuf));
+	}
+	release_table(table);
+
+	ast_str_append(&sql, 0, " %s", ast_str_buffer(where));
+
+	ast_debug(1, "PostgreSQL RealTime: Update SQL: %s\n", ast_str_buffer(sql));
+
+	/* We now have our complete statement; connect to the server and execute it. */
+	ast_mutex_lock(&pgsql_lock);
+	if (!pgsql_reconnect(database)) {
+		ast_mutex_unlock(&pgsql_lock);
+		return -1;
+	}
+
+	if (!(result = PQexec(pgsqlConn, ast_str_buffer(sql)))) {
+		ast_log(LOG_WARNING,
+				"PostgreSQL RealTime: Failed to query database. Check debug for more info.\n");
+		ast_debug(1, "PostgreSQL RealTime: Query: %s\n", ast_str_buffer(sql));
+		ast_debug(1, "PostgreSQL RealTime: Query Failed because: %s\n", PQerrorMessage(pgsqlConn));
+		ast_mutex_unlock(&pgsql_lock);
+		return -1;
+	} else {
+		ExecStatusType result_status = PQresultStatus(result);
+		if (result_status != PGRES_COMMAND_OK
+			&& result_status != PGRES_TUPLES_OK
+			&& result_status != PGRES_NONFATAL_ERROR) {
+			ast_log(LOG_WARNING,
+					"PostgreSQL RealTime: Failed to query database. Check debug for more info.\n");
+			ast_debug(1, "PostgreSQL RealTime: Query: %s\n", ast_str_buffer(sql));
+			ast_debug(1, "PostgreSQL RealTime: Query Failed because: %s (%s)\n",
+						PQresultErrorMessage(result), PQresStatus(result_status));
+			ast_mutex_unlock(&pgsql_lock);
+			return -1;
+		}
+	}
+
+	numrows = atoi(PQcmdTuples(result));
+	ast_mutex_unlock(&pgsql_lock);
+
+	ast_debug(1, "PostgreSQL RealTime: Updated %d rows on table: %s\n", numrows, tablename);
+
+	/* From http://dev.pgsql.com/doc/pgsql/en/pgsql-affected-rows.html
+	 * An integer greater than zero indicates the number of rows affected
+	 * Zero indicates that no records were updated
+	 * -1 indicates that the query returned an error (although, if the query failed, it should have been caught above.)
+	 */
+
 	if (numrows >= 0) {
 		return (int) numrows;
 	}
@@ -893,7 +882,7 @@
 
 static int store_pgsql(const char *database, const char *table, va_list ap)
 {
-	PGresult *result = NULL;
+	RAII_VAR(PGresult *, result, NULL, PQclear);
 	Oid insertid;
 	struct ast_str *buf = ast_str_thread_get(&escapebuf_buf, 256);
 	struct ast_str *sql1 = ast_str_thread_get(&sql_buf, 256);
@@ -972,7 +961,6 @@
 	}
 
 	insertid = PQoidValue(result);
-	PQclear(result);
 	ast_mutex_unlock(&pgsql_lock);
 
 	ast_debug(1, "PostgreSQL RealTime: row inserted on table: %s, id: %u\n", table, insertid);
@@ -991,7 +979,7 @@
 
 static int destroy_pgsql(const char *database, const char *table, const char *keyfield, const char *lookup, va_list ap)
 {
-	PGresult *result = NULL;
+	RAII_VAR(PGresult *, result, NULL, PQclear);
 	int numrows = 0;
 	int pgresult;
 	struct ast_str *sql = ast_str_thread_get(&sql_buf, 256);
@@ -1090,7 +1078,7 @@
 									   const char *file, struct ast_config *cfg,
 									   struct ast_flags flags, const char *suggested_incl, const char *who_asked)
 {
-	PGresult *result = NULL;
+	RAII_VAR(PGresult *, result, NULL, PQclear);
 	long num_rows;
 	struct ast_variable *new_v;
 	struct ast_category *cur_cat = NULL;
@@ -1158,7 +1146,6 @@
 			char *field_cat_metric = PQgetvalue(result, rowIndex, 3);
 			if (!strcmp(field_var_name, "#include")) {
 				if (!ast_config_internal_load(field_var_val, cfg, flags, "", who_asked)) {
-					PQclear(result);
 					ast_mutex_unlock(&pgsql_lock);
 					return NULL;
 				}
@@ -1181,7 +1168,6 @@
 				"PostgreSQL RealTime: Could not find config '%s' in database.\n", file);
 	}
 
-	PQclear(result);
 	ast_mutex_unlock(&pgsql_lock);
 
 	return cfg;




More information about the asterisk-commits mailing list