[asterisk-commits] mjordan: trunk r348890 - in /trunk: ./ cel/cel_pgsql.c

[SVN commits to the Asterisk project]

Author: mjordan
Date: Thu Dec 22 16:39:29 2011
New Revision: 348890

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=348890
Log:
Fix for memory leaks / cleanup in cel_pgsql

There were a number of issues in cel_pgsql's pgsql_log method:
* If either sql or sql2 could not be allocated, the method would return while
the pgsql_lock was still locked
* If the execution of the log statement succeeded, the sql and sql2 structs
were never free'd
* Reconnection successes were logged as ERRORs.  In general, the severity of
several logging statements was reduced

(closes issue ASTERISK-18879)
Reported by: Niolas Bouliane
Tested by: Matt Jordan

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

Merged revisions 348888 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 348889 from http://svn.asterisk.org/svn/asterisk/branches/10

Modified:
    trunk/   (props changed)
    trunk/cel/cel_pgsql.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.

Modified: trunk/cel/cel_pgsql.c
URL: http://svnview.digium.com/svn/asterisk/trunk/cel/cel_pgsql.c?view=diff&rev=348890&r1=348889&r2=348890
==============================================================================
--- trunk/cel/cel_pgsql.c (original)
+++ trunk/cel/cel_pgsql.c Thu Dec 22 16:39:29 2011
@@ -145,13 +145,7 @@
 		int first = 1;
 
 		if (!sql || !sql2) {
-			if (sql) {
-				ast_free(sql);
-			}
-			if (sql2) {
-				ast_free(sql2);
-			}
-			return;
+			goto ast_log_cleanup;
 		}
 
 		ast_str_set(&sql, 0, "INSERT INTO %s (", table);
@@ -291,10 +285,10 @@
 		if (PQstatus(conn) == CONNECTION_OK) {
 			connected = 1;
 		} else {
-			ast_log(LOG_ERROR, "Connection was lost... attempting to reconnect.\n");
+			ast_log(LOG_WARNING, "Connection was lost... attempting to reconnect.\n");
 			PQreset(conn);
 			if (PQstatus(conn) == CONNECTION_OK) {
-				ast_log(LOG_ERROR, "Connection reestablished.\n");
+				ast_log(LOG_NOTICE, "Connection reestablished.\n");
 				connected = 1;
 			} else {
 				pgerror = PQerrorMessage(conn);
@@ -303,21 +297,18 @@
 				PQfinish(conn);
 				conn = NULL;
 				connected = 0;
-				ast_mutex_unlock(&pgsql_lock);
-				ast_free(sql);
-				ast_free(sql2);
-				return;
+				goto ast_log_cleanup;
 			}
 		}
 		result = PQexec(conn, ast_str_buffer(sql));
 		if (PQresultStatus(result) != PGRES_COMMAND_OK) {
 			pgerror = PQresultErrorMessage(result);
-			ast_log(LOG_ERROR, "Failed to insert call detail record into database!\n");
-			ast_log(LOG_ERROR, "Reason: %s\n", pgerror);
-			ast_log(LOG_ERROR, "Connection may have been lost... attempting to reconnect.\n");
+			ast_log(LOG_WARNING, "Failed to insert call detail record into database!\n");
+			ast_log(LOG_WARNING, "Reason: %s\n", pgerror);
+			ast_log(LOG_WARNING, "Connection may have been lost... attempting to reconnect.\n");
 			PQreset(conn);
 			if (PQstatus(conn) == CONNECTION_OK) {
-				ast_log(LOG_ERROR, "Connection reestablished.\n");
+				ast_log(LOG_NOTICE, "Connection reestablished.\n");
 				connected = 1;
 				PQclear(result);
 				result = PQexec(conn, ast_str_buffer(sql));
@@ -327,14 +318,16 @@
 					ast_log(LOG_ERROR, "Reason: %s\n", pgerror);
 				}
 			}
-			ast_mutex_unlock(&pgsql_lock);
 			PQclear(result);
-			ast_free(sql);
-			ast_free(sql2);
-			return;
-		}
-		ast_mutex_unlock(&pgsql_lock);
-	}
+			goto ast_log_cleanup;
+		}
+
+ast_log_cleanup:
+		ast_free(sql);
+		ast_free(sql2);
+	}
+
+	ast_mutex_unlock(&pgsql_lock);
 }
 
 static int my_unload_module(void)