[Asterisk-code-review] cdr mysql: split mysql init out of my load module (asterisk[master])

George Joseph asteriskteam at digium.com
Wed Jun 6 08:12:39 CDT 2018


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/8919 )

Change subject: cdr_mysql: split mysql init out of my_load_module
......................................................................

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
---
M addons/cdr_mysql.c
1 file changed, 134 insertions(+), 127 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  Matthew Fredrickson: Looks good to me, approved
  George Joseph: Approved for Submit



diff --git a/addons/cdr_mysql.c b/addons/cdr_mysql.c
index 97ebdf2..b194b7e 100644
--- a/addons/cdr_mysql.c
+++ b/addons/cdr_mysql.c
@@ -448,6 +448,137 @@
 	return 0;
 }
 
+/** 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)
+{
+	struct ast_variable *var;
+	char *temp;
+	MYSQL_ROW row;
+	MYSQL_RES *result;
+	char sqldesc[128];
+#if MYSQL_VERSION_ID >= 50013
+	my_bool my_bool_true = 1;
+#endif
+
+	mysql_init(&mysql);
+
+	if (timeout && mysql_options(&mysql, MYSQL_OPT_CONNECT_TIMEOUT, (char *)&timeout) != 0) {
+		ast_log(LOG_ERROR, "cdr_mysql: mysql_options returned (%d) %s\n", mysql_errno(&mysql), mysql_error(&mysql));
+	}
+
+#if MYSQL_VERSION_ID >= 50013
+	/* Add option for automatic reconnection */
+	if (mysql_options(&mysql, MYSQL_OPT_RECONNECT, &my_bool_true) != 0) {
+		ast_log(LOG_ERROR, "cdr_mysql: mysql_options returned (%d) %s\n", mysql_errno(&mysql), mysql_error(&mysql));
+	}
+#endif
+
+	if ((ssl_ca && ast_str_strlen(ssl_ca)) || (ssl_cert && ast_str_strlen(ssl_cert)) || (ssl_key && ast_str_strlen(ssl_key))) {
+		mysql_ssl_set(&mysql,
+			ssl_key ? ast_str_buffer(ssl_key) : NULL,
+			ssl_cert ? ast_str_buffer(ssl_cert) : NULL,
+			ssl_ca ? ast_str_buffer(ssl_ca) : NULL,
+			NULL, NULL);
+	}
+	temp = dbsock && ast_str_strlen(dbsock) ? ast_str_buffer(dbsock) : NULL;
+
+	configure_connection_charset();
+
+	if (!mysql_real_connect(&mysql, ast_str_buffer(hostname), ast_str_buffer(dbuser), ast_str_buffer(password), ast_str_buffer(dbname), dbport, temp, ssl_ca && ast_str_strlen(ssl_ca) ? CLIENT_SSL : 0)) {
+		ast_log(LOG_ERROR, "Failed to connect to mysql database %s on %s.\n", ast_str_buffer(dbname), ast_str_buffer(hostname));
+		connected = 0;
+		records = 0;
+	} else {
+		ast_debug(1, "Successfully connected to MySQL database.\n");
+		connected = 1;
+		records = 0;
+		connect_time = time(NULL);
+
+		/* Get table description */
+		snprintf(sqldesc, sizeof(sqldesc), "DESC %s", dbtable ? ast_str_buffer(dbtable) : "cdr");
+		if (mysql_query(&mysql, sqldesc)) {
+			ast_log(LOG_ERROR, "Unable to query table description!!  Logging disabled.\n");
+			mysql_close(&mysql);
+			connected = 0;
+
+			return AST_MODULE_LOAD_DECLINE;
+		}
+
+		if (!(result = mysql_store_result(&mysql))) {
+			ast_log(LOG_ERROR, "Unable to query table description!!  Logging disabled.\n");
+			mysql_close(&mysql);
+			connected = 0;
+
+			return AST_MODULE_LOAD_DECLINE;
+		}
+
+		while ((row = mysql_fetch_row(result))) {
+			struct column *entry;
+			char *cdrvar = "", *staticvalue = "";
+
+			ast_debug(1, "Got a field '%s' of type '%s'\n", row[0], row[1]);
+			/* Check for an alias or a static value */
+			for (var = ast_variable_browse(cfg, "columns"); var; var = var->next) {
+				if (strncmp(var->name, "alias", 5) == 0 && strcasecmp(var->value, row[0]) == 0 ) {
+					char *alias = ast_strdupa(var->name + 5);
+					cdrvar = ast_strip(alias);
+					ast_verb(3, "Found alias %s for column %s\n", cdrvar, row[0]);
+					break;
+				} else if (strncmp(var->name, "static", 6) == 0 && strcasecmp(var->value, row[0]) == 0) {
+					char *item = ast_strdupa(var->name + 6);
+					item = ast_strip(item);
+					if (item[0] == '"' && item[strlen(item) - 1] == '"') {
+						/* Remove surrounding quotes */
+						item[strlen(item) - 1] = '\0';
+						item++;
+					}
+					staticvalue = item;
+				}
+			}
+
+			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]);
+				mysql_free_result(result);
+				return AST_MODULE_LOAD_DECLINE;
+			}
+
+			entry->name = (char *)entry + sizeof(*entry);
+			strcpy(entry->name, row[0]);
+
+			if (!ast_strlen_zero(cdrvar)) {
+				entry->cdrname = entry->name + strlen(row[0]) + 1;
+				strcpy(entry->cdrname, cdrvar);
+			} else { /* Point to same place as the column name */
+				entry->cdrname = (char *)entry + sizeof(*entry);
+			}
+
+			if (!ast_strlen_zero(staticvalue)) {
+				entry->staticvalue = entry->cdrname + strlen(entry->cdrname) + 1;
+				strcpy(entry->staticvalue, staticvalue);
+				ast_debug(1, "staticvalue length: %d\n", (int) strlen(staticvalue) );
+				entry->type = entry->staticvalue + strlen(entry->staticvalue) + 1;
+			} else {
+				entry->type = entry->cdrname + strlen(entry->cdrname) + 1;
+			}
+			strcpy(entry->type, row[1]);
+
+			ast_debug(1, "Entry name '%s'\n", entry->name);
+			ast_debug(1, "   cdrname '%s'\n", entry->cdrname);
+			ast_debug(1, "    static '%s'\n", entry->staticvalue);
+			ast_debug(1, "      type '%s'\n", entry->type);
+
+			AST_LIST_INSERT_TAIL(&columns, entry, list);
+		}
+		mysql_free_result(result);
+	}
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
 static int my_load_module(int reload)
 {
 	int res;
@@ -458,14 +589,7 @@
 	 * 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];
-#if MYSQL_VERSION_ID >= 50013
-	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,
@@ -562,129 +686,12 @@
 		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) {
-		ast_log(LOG_ERROR, "cdr_mysql: mysql_options returned (%d) %s\n", mysql_errno(&mysql), mysql_error(&mysql));
-	}
-
-#if MYSQL_VERSION_ID >= 50013
-	/* Add option for automatic reconnection */
-	if (mysql_options(&mysql, MYSQL_OPT_RECONNECT, &my_bool_true) != 0) {
-		ast_log(LOG_ERROR, "cdr_mysql: mysql_options returned (%d) %s\n", mysql_errno(&mysql), mysql_error(&mysql));
-	}
-#endif
-
-	if ((ssl_ca && ast_str_strlen(ssl_ca)) || (ssl_cert && ast_str_strlen(ssl_cert)) || (ssl_key && ast_str_strlen(ssl_key))) {
-		mysql_ssl_set(&mysql,
-			ssl_key ? ast_str_buffer(ssl_key) : NULL,
-			ssl_cert ? ast_str_buffer(ssl_cert) : NULL,
-			ssl_ca ? ast_str_buffer(ssl_ca) : NULL,
-			NULL, NULL);
-	}
-	temp = dbsock && ast_str_strlen(dbsock) ? ast_str_buffer(dbsock) : NULL;
-
-	configure_connection_charset();
-
-	if (!mysql_real_connect(&mysql, ast_str_buffer(hostname), ast_str_buffer(dbuser), ast_str_buffer(password), ast_str_buffer(dbname), dbport, temp, ssl_ca && ast_str_strlen(ssl_ca) ? CLIENT_SSL : 0)) {
-		ast_log(LOG_ERROR, "Failed to connect to mysql database %s on %s.\n", ast_str_buffer(dbname), ast_str_buffer(hostname));
-		connected = 0;
-		records = 0;
-	} else {
-		ast_debug(1, "Successfully connected to MySQL database.\n");
-		connected = 1;
-		records = 0;
-		connect_time = time(NULL);
-
-		/* Get table description */
-		snprintf(sqldesc, sizeof(sqldesc), "DESC %s", dbtable ? ast_str_buffer(dbtable) : "cdr");
-		if (mysql_query(&mysql, sqldesc)) {
-			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;
-		}
-
-		if (!(result = mysql_store_result(&mysql))) {
-			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;
-		}
-
-		while ((row = mysql_fetch_row(result))) {
-			struct column *entry;
-			char *cdrvar = "", *staticvalue = "";
-
-			ast_debug(1, "Got a field '%s' of type '%s'\n", row[0], row[1]);
-			/* Check for an alias or a static value */
-			for (var = ast_variable_browse(cfg, "columns"); var; var = var->next) {
-				if (strncmp(var->name, "alias", 5) == 0 && strcasecmp(var->value, row[0]) == 0 ) {
-					char *alias = ast_strdupa(var->name + 5);
-					cdrvar = ast_strip(alias);
-					ast_verb(3, "Found alias %s for column %s\n", cdrvar, row[0]);
-					break;
-				} else if (strncmp(var->name, "static", 6) == 0 && strcasecmp(var->value, row[0]) == 0) {
-					char *item = ast_strdupa(var->name + 6);
-					item = ast_strip(item);
-					if (item[0] == '"' && item[strlen(item) - 1] == '"') {
-						/* Remove surrounding quotes */
-						item[strlen(item) - 1] = '\0';
-						item++;
-					}
-					staticvalue = item;
-				}
-			}
-
-			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;
-			}
-
-			entry->name = (char *)entry + sizeof(*entry);
-			strcpy(entry->name, row[0]);
-
-			if (!ast_strlen_zero(cdrvar)) {
-				entry->cdrname = entry->name + strlen(row[0]) + 1;
-				strcpy(entry->cdrname, cdrvar);
-			} else { /* Point to same place as the column name */
-				entry->cdrname = (char *)entry + sizeof(*entry);
-			}
-
-			if (!ast_strlen_zero(staticvalue)) {
-				entry->staticvalue = entry->cdrname + strlen(entry->cdrname) + 1;
-				strcpy(entry->staticvalue, staticvalue);
-				ast_debug(1, "staticvalue length: %d\n", (int) strlen(staticvalue) );
-				entry->type = entry->staticvalue + strlen(entry->staticvalue) + 1;
-			} else {
-				entry->type = entry->cdrname + strlen(entry->cdrname) + 1;
-			}
-			strcpy(entry->type, row[1]);
-
-			ast_debug(1, "Entry name '%s'\n", entry->name);
-			ast_debug(1, "   cdrname '%s'\n", entry->cdrname);
-			ast_debug(1, "    static '%s'\n", entry->staticvalue);
-			ast_debug(1, "      type '%s'\n", entry->type);
-
-			AST_LIST_INSERT_TAIL(&columns, entry, list);
-		}
-		mysql_free_result(result);
-	}
+	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) {

-- 
To view, visit https://gerrit.asterisk.org/8919
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If2ee676056067cc693ff08be68ee4944bf35b49f
Gerrit-Change-Number: 8919
Gerrit-PatchSet: 2
Gerrit-Owner: Tzafrir Cohen <tzafrir.cohen at xorcom.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Oron Peled <oron.peled at xorcom.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180606/461cee84/attachment-0001.html>


More information about the asterisk-code-review mailing list