[PATCH ulogd2 v2 v2 32/34] output: pgsql: tidy up `open_db_pgsql` and fix memory leak

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 * Remove some excess parentheses and a superfluous local variable.
 * Tighten up the scope of a couple of other variables.
 * Streamline the error-handling.
 * Free the connexion string.

Signed-off-by: Jeremy Sowden <jeremy@xxxxxxxxxx>
---
 output/pgsql/ulogd_output_PGSQL.c | 57 +++++++++++++++++--------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/output/pgsql/ulogd_output_PGSQL.c b/output/pgsql/ulogd_output_PGSQL.c
index b125674b7364..8c9aabf873ca 100644
--- a/output/pgsql/ulogd_output_PGSQL.c
+++ b/output/pgsql/ulogd_output_PGSQL.c
@@ -232,11 +232,9 @@ static int close_db_pgsql(struct ulogd_pluginstance *upi)
 static int open_db_pgsql(struct ulogd_pluginstance *upi)
 {
 	struct pgsql_instance *pi = (struct pgsql_instance *) upi->private;
-	int len;
 	char *connstr = connstr_ce(upi->config_kset).u.string;
-	char *schema = NULL;
-	char pgbuf[128];
-	PGresult *pgres;
+	PGresult *pgres = NULL;
+	int rv = -1;
 
 	if (!connstr[0]) {
 		char         *server = host_ce(upi->config_kset).u.string;
@@ -245,10 +243,11 @@ static int open_db_pgsql(struct ulogd_pluginstance *upi)
 		char         *pass   = pass_ce(upi->config_kset).u.string;
 		char         *db     = db_ce(upi->config_kset).u.string;
 		char         *cp;
+
 		/* 80 is more than what we need for the fixed parts below */
-		len = 80 + strlen(user) + strlen(db);
+		size_t len = 80 + strlen(user) + strlen(db);
 
-		/* hostname and  and password are the only optionals */
+		/* hostname and and password are the only optionals */
 		if (server[0])
 			len += strlen(server);
 		if (pass[0])
@@ -271,39 +270,47 @@ static int open_db_pgsql(struct ulogd_pluginstance *upi)
 		if (pass[0])
 			cp += sprintf(cp, " password=%s", pass);
 	}
+
 	pi->dbh = PQconnectdb(connstr);
 	if (PQstatus(pi->dbh) != CONNECTION_OK) {
 		ulogd_log(ULOGD_ERROR, "unable to connect to db (%s): %s\n",
 			  connstr, PQerrorMessage(pi->dbh));
-		close_db_pgsql(upi);
-		return -1;
+		goto err_close_db;
 	}
 
-	if (pgsql_namespace(upi)) {
+	if (pgsql_namespace(upi) < 0) {
 		ulogd_log(ULOGD_ERROR, "problem testing for pgsql schemas\n");
-		close_db_pgsql(upi);
-		return -1;
+		goto err_close_db;
 	}
 
-	pi=(struct pgsql_instance *)upi->private;
-	schema = pi->db_inst.schema;
+	if (pi->db_inst.schema != NULL && strcmp(pi->db_inst.schema,"public") != 0) {
+		char pgbuf[128];
 
-	if (!(schema == NULL) && (strcmp(schema,"public"))) {
-		snprintf(pgbuf, 128, "SET search_path='%.63s', \"$user\", 'public'", schema);
+		snprintf(pgbuf, sizeof(pgbuf),
+			 "SET search_path='%.63s', \"$user\", 'public'",
+			 pi->db_inst.schema);
 		pgres = PQexec(pi->dbh, pgbuf);
-		if ((PQresultStatus(pgres) == PGRES_COMMAND_OK)) {
-			PQclear(pgres);
-		} else {
-			ulogd_log(ULOGD_ERROR, "could not set search path to (%s): %s\n",
-				 schema, PQerrorMessage(pi->dbh));
-			PQclear(pgres);
-			close_db_pgsql(upi);
-			return -1;
+		if (PQresultStatus(pgres) != PGRES_COMMAND_OK) {
+			ulogd_log(ULOGD_ERROR,
+				  "could not set search path to (%s): %s\n",
+				  pi->db_inst.schema, PQerrorMessage(pi->dbh));
+			goto err_free_result;
 		}
-
 	}
 
-	return 0;
+	rv = 0;
+
+err_free_result:
+	PQclear(pgres);
+
+err_close_db:
+	if (rv == -1)
+		close_db_pgsql(upi);
+
+	if (connstr != connstr_ce(upi->config_kset).u.string)
+		free(connstr);
+
+	return rv;
 }
 
 static int escape_string_pgsql(struct ulogd_pluginstance *upi,
-- 
2.35.1




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux