From 2e44adf1c3da5ee34118bee8a23c25b39fd387ab Mon Sep 17 00:00:00 2001
From: Tzafrir Cohen <tzafrir.cohen@xorcom.com>
Date: Thu, 11 Jan 2018 14:33:08 +0200
Subject: [PATCH] cdr_mysql: split mysql init out of my_load_module

Split out mysql connection parts to a separate my_connect_db().

ASTERISK-27572

Change-Id: If2ee676056067cc693ff08be68ee4944bf35b49f
---
 addons/cdr_mysql.c | 235 +++++++++++++++++++++++----------------------
 1 file changed, 121 insertions(+), 114 deletions(-)

diff --git a/addons/cdr_mysql.c b/addons/cdr_mysql.c
index 97ebdf26f4..b194b7e4c2 100644
--- a/addons/cdr_mysql.c
+++ b/addons/cdr_mysql.c
@@ -448,18 +448,15 @@ static int my_load_config_number(struct ast_config *cfg, const char *category, c
 	return 0;
 }
 
-static int my_load_module(int reload)
+/** Connect to MySQL. Initializes the connection.
+ *
+ * * Assumes the read-write lock for columns is held.
+ * * Caller should allocate and free cfg
+ * */
+static int my_connect_db(struct ast_config *cfg)
 {
-	int res;
-	struct ast_config *cfg;
 	struct ast_variable *var;
-	/* CONFIG_STATUS_FILEUNCHANGED is impossible when config_flags is always 0,
-	 * and it has to be zero, so a reload can be sent to tell the driver to
-	 * rescan the table layout. */
-	struct ast_flags config_flags = { 0 };
-	struct column *entry;
 	char *temp;
-	struct ast_str *compat;
 	MYSQL_ROW row;
 	MYSQL_RES *result;
 	char sqldesc[128];
@@ -467,101 +464,6 @@ static int my_load_module(int reload)
 	my_bool my_bool_true = 1;
 #endif
 
-	/* Cannot use a conditionally different flag, because the table layout may
-	 * have changed, which is not detectable by config file change detection,
-	 * but should still cause the configuration to be re-parsed. */
-	cfg = ast_config_load(config, config_flags);
-	if (cfg == CONFIG_STATUS_FILEMISSING) {
-		ast_log(LOG_WARNING, "Unable to load config for mysql CDR's: %s\n", config);
-		return AST_MODULE_LOAD_SUCCESS;
-	} else if (cfg == CONFIG_STATUS_FILEINVALID) {
-		ast_log(LOG_ERROR, "Unable to load configuration file '%s'\n", config);
-		return AST_MODULE_LOAD_DECLINE;
-	}
-
-	if (reload) {
-		AST_RWLIST_WRLOCK(&columns);
-		my_unload_module(1);
-	}
-
-	var = ast_variable_browse(cfg, "global");
-	if (!var) {
-		/* nothing configured */
-		if (reload) {
-			AST_RWLIST_UNLOCK(&columns);
-		}
-		ast_config_destroy(cfg);
-		return AST_MODULE_LOAD_SUCCESS;
-	}
-
-	res = 0;
-
-	res |= my_load_config_string(cfg, "global", "hostname", &hostname, "localhost");
-	res |= my_load_config_string(cfg, "global", "dbname", &dbname, "astriskcdrdb");
-	res |= my_load_config_string(cfg, "global", "user", &dbuser, "root");
-	res |= my_load_config_string(cfg, "global", "sock", &dbsock, "");
-	res |= my_load_config_string(cfg, "global", "table", &dbtable, "cdr");
-	res |= my_load_config_string(cfg, "global", "password", &password, "");
-
-	res |= my_load_config_string(cfg, "global", "charset", &dbcharset, "");
-
-	res |= my_load_config_string(cfg, "global", "ssl_ca", &ssl_ca, "");
-	res |= my_load_config_string(cfg, "global", "ssl_cert", &ssl_cert, "");
-	res |= my_load_config_string(cfg, "global", "ssl_key", &ssl_key, "");
-
-	res |= my_load_config_number(cfg, "global", "port", &dbport, MYSQL_PORT);
-	res |= my_load_config_number(cfg, "global", "timeout", &timeout, 0);
-	res |= my_load_config_string(cfg, "global", "compat", &compat, "no");
-	res |= my_load_config_string(cfg, "global", "cdrzone", &cdrzone, "");
-	if (ast_str_strlen(cdrzone) == 0) {
-		for (; var; var = var->next) {
-			if (!strcasecmp(var->name, "usegmtime") && ast_true(var->value)) {
-				ast_str_set(&cdrzone, 0, "UTC");
-			}
-		}
-	}
-
-	if (ast_true(ast_str_buffer(compat))) {
-		calldate_compat = 1;
-	} else {
-		calldate_compat = 0;
-	}
-
-	if (res < 0) {
-		if (reload) {
-			AST_RWLIST_UNLOCK(&columns);
-		}
-		ast_config_destroy(cfg);
-		free_strings();
-
-		return AST_MODULE_LOAD_DECLINE;
-	}
-
-	/* Check for any aliases */
-	if (!reload) {
-		/* Lock, if not already */
-		AST_RWLIST_WRLOCK(&columns);
-	}
-	while ((entry = AST_LIST_REMOVE_HEAD(&columns, list))) {
-		ast_free(entry);
-	}
-
-	ast_debug(1, "Got hostname of %s\n", ast_str_buffer(hostname));
-	ast_debug(1, "Got port of %d\n", dbport);
-	ast_debug(1, "Got a timeout of %d\n", timeout);
-	if (ast_str_strlen(dbsock)) {
-		ast_debug(1, "Got sock file of %s\n", ast_str_buffer(dbsock));
-	}
-	ast_debug(1, "Got user of %s\n", ast_str_buffer(dbuser));
-	ast_debug(1, "Got dbname of %s\n", ast_str_buffer(dbname));
-	ast_debug(1, "Got password of %s\n", ast_str_buffer(password));
-	ast_debug(1, "%sunning in calldate compatibility mode\n", calldate_compat ? "R" : "Not r");
-	ast_debug(1, "Dates and times are localized to %s\n", S_OR(ast_str_buffer(cdrzone), "local timezone"));
-
-	if (ast_str_strlen(dbcharset)) {
-		ast_debug(1, "Got DB charset of %s\n", ast_str_buffer(dbcharset));
-	}
-
 	mysql_init(&mysql);
 
 	if (timeout && mysql_options(&mysql, MYSQL_OPT_CONNECT_TIMEOUT, (char *)&timeout) != 0) {
@@ -602,9 +504,6 @@ static int my_load_module(int reload)
 			ast_log(LOG_ERROR, "Unable to query table description!!  Logging disabled.\n");
 			mysql_close(&mysql);
 			connected = 0;
-			AST_RWLIST_UNLOCK(&columns);
-			ast_config_destroy(cfg);
-			free_strings();
 
 			return AST_MODULE_LOAD_DECLINE;
 		}
@@ -613,9 +512,6 @@ static int my_load_module(int reload)
 			ast_log(LOG_ERROR, "Unable to query table description!!  Logging disabled.\n");
 			mysql_close(&mysql);
 			connected = 0;
-			AST_RWLIST_UNLOCK(&columns);
-			ast_config_destroy(cfg);
-			free_strings();
 
 			return AST_MODULE_LOAD_DECLINE;
 		}
@@ -647,8 +543,8 @@ static int my_load_module(int reload)
 			entry = ast_calloc(sizeof(char), sizeof(*entry) + strlen(row[0]) + 1 + strlen(cdrvar) + 1 + strlen(staticvalue) + 1 + strlen(row[1]) + 1);
 			if (!entry) {
 				ast_log(LOG_ERROR, "Out of memory creating entry for column '%s'\n", row[0]);
-				res = -1;
-				break;
+				mysql_free_result(result);
+				return AST_MODULE_LOAD_DECLINE;
 			}
 
 			entry->name = (char *)entry + sizeof(*entry);
@@ -680,11 +576,122 @@ static int my_load_module(int reload)
 		}
 		mysql_free_result(result);
 	}
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
+static int my_load_module(int reload)
+{
+	int res;
+	struct ast_config *cfg;
+	struct ast_variable *var;
+	/* CONFIG_STATUS_FILEUNCHANGED is impossible when config_flags is always 0,
+	 * and it has to be zero, so a reload can be sent to tell the driver to
+	 * rescan the table layout. */
+	struct ast_flags config_flags = { 0 };
+	struct column *entry;
+	struct ast_str *compat;
+
+	/* Cannot use a conditionally different flag, because the table layout may
+	 * have changed, which is not detectable by config file change detection,
+	 * but should still cause the configuration to be re-parsed. */
+	cfg = ast_config_load(config, config_flags);
+	if (cfg == CONFIG_STATUS_FILEMISSING) {
+		ast_log(LOG_WARNING, "Unable to load config for mysql CDR's: %s\n", config);
+		return AST_MODULE_LOAD_SUCCESS;
+	} else if (cfg == CONFIG_STATUS_FILEINVALID) {
+		ast_log(LOG_ERROR, "Unable to load configuration file '%s'\n", config);
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
+	if (reload) {
+		AST_RWLIST_WRLOCK(&columns);
+		my_unload_module(1);
+	}
+
+	var = ast_variable_browse(cfg, "global");
+	if (!var) {
+		/* nothing configured */
+		if (reload) {
+			AST_RWLIST_UNLOCK(&columns);
+		}
+		ast_config_destroy(cfg);
+		return AST_MODULE_LOAD_SUCCESS;
+	}
+
+	res = 0;
+
+	res |= my_load_config_string(cfg, "global", "hostname", &hostname, "localhost");
+	res |= my_load_config_string(cfg, "global", "dbname", &dbname, "astriskcdrdb");
+	res |= my_load_config_string(cfg, "global", "user", &dbuser, "root");
+	res |= my_load_config_string(cfg, "global", "sock", &dbsock, "");
+	res |= my_load_config_string(cfg, "global", "table", &dbtable, "cdr");
+	res |= my_load_config_string(cfg, "global", "password", &password, "");
+
+	res |= my_load_config_string(cfg, "global", "charset", &dbcharset, "");
+
+	res |= my_load_config_string(cfg, "global", "ssl_ca", &ssl_ca, "");
+	res |= my_load_config_string(cfg, "global", "ssl_cert", &ssl_cert, "");
+	res |= my_load_config_string(cfg, "global", "ssl_key", &ssl_key, "");
+
+	res |= my_load_config_number(cfg, "global", "port", &dbport, MYSQL_PORT);
+	res |= my_load_config_number(cfg, "global", "timeout", &timeout, 0);
+	res |= my_load_config_string(cfg, "global", "compat", &compat, "no");
+	res |= my_load_config_string(cfg, "global", "cdrzone", &cdrzone, "");
+	if (ast_str_strlen(cdrzone) == 0) {
+		for (; var; var = var->next) {
+			if (!strcasecmp(var->name, "usegmtime") && ast_true(var->value)) {
+				ast_str_set(&cdrzone, 0, "UTC");
+			}
+		}
+	}
+
+	if (ast_true(ast_str_buffer(compat))) {
+		calldate_compat = 1;
+	} else {
+		calldate_compat = 0;
+	}
+
+	if (res < 0) {
+		if (reload) {
+			AST_RWLIST_UNLOCK(&columns);
+		}
+		ast_config_destroy(cfg);
+		free_strings();
+
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
+	/* Check for any aliases */
+	if (!reload) {
+		/* Lock, if not already */
+		AST_RWLIST_WRLOCK(&columns);
+	}
+	while ((entry = AST_LIST_REMOVE_HEAD(&columns, list))) {
+		ast_free(entry);
+	}
+
+	ast_debug(1, "Got hostname of %s\n", ast_str_buffer(hostname));
+	ast_debug(1, "Got port of %d\n", dbport);
+	ast_debug(1, "Got a timeout of %d\n", timeout);
+	if (ast_str_strlen(dbsock)) {
+		ast_debug(1, "Got sock file of %s\n", ast_str_buffer(dbsock));
+	}
+	ast_debug(1, "Got user of %s\n", ast_str_buffer(dbuser));
+	ast_debug(1, "Got dbname of %s\n", ast_str_buffer(dbname));
+	ast_debug(1, "Got password of %s\n", ast_str_buffer(password));
+	ast_debug(1, "%sunning in calldate compatibility mode\n", calldate_compat ? "R" : "Not r");
+	ast_debug(1, "Dates and times are localized to %s\n", S_OR(ast_str_buffer(cdrzone), "local timezone"));
+
+	if (ast_str_strlen(dbcharset)) {
+		ast_debug(1, "Got DB charset of %s\n", ast_str_buffer(dbcharset));
+	}
+
+	res = my_connect_db(cfg);
 	AST_RWLIST_UNLOCK(&columns);
 	ast_config_destroy(cfg);
-	if (res < 0) {
+	if (res != AST_MODULE_LOAD_SUCCESS) {
 		my_unload_module(0);
-		return AST_MODULE_LOAD_DECLINE;
+		return res;
 	}
 
 	if (!reload) {