[Asterisk-code-review] Refactor init logger chain locking. (asterisk[master])

[Mark Michelson]

Mark Michelson has submitted this change and it was merged.

Change subject: Refactor init_logger_chain locking.
......................................................................


Refactor init_logger_chain locking.

This removes logchannels locking from init_logger_chain, puts the
responsibility on the caller.  Adds locking around the one call that was
missing it.

ASTERISK-24833

Change-Id: I6cc42117338bf9575650a67bcb78ab1a33d7bad8
---
M main/logger.c
1 file changed, 8 insertions(+), 25 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Richard Mudgett: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified



diff --git a/main/logger.c b/main/logger.c
index 73b8ee1..fac68d9 100644
--- a/main/logger.c
+++ b/main/logger.c
@@ -560,13 +560,14 @@
 }
 
 /* \brief Read config, setup channels.
- * \param locked The logchannels list is locked and this is a reload
  * \param altconf Alternate configuration file to read.
+ *
+ * \pre logchannels list is write locked
  *
  * \retval 0 Success
  * \retval -1 No config found or Failed
  */
-static int init_logger_chain(int locked, const char *altconf)
+static int init_logger_chain(const char *altconf)
 {
 	struct logchannel *chan;
 	struct ast_config *cfg;
@@ -576,10 +577,6 @@
 
 	if (!(cfg = ast_config_load2(S_OR(altconf, "logger.conf"), "logger", config_flags)) || cfg == CONFIG_STATUS_FILEINVALID) {
 		cfg = NULL;
-	}
-
-	if (!locked) {
-		AST_RWLIST_WRLOCK(&logchannels);
 	}
 
 	/* Set defaults */
@@ -597,9 +594,6 @@
 		ast_free(chan);
 	}
 	global_logmask = 0;
-	if (!locked) {
-		AST_RWLIST_UNLOCK(&logchannels);
-	}
 
 	errno = 0;
 	/* close syslog */
@@ -615,14 +609,8 @@
 		chan->logmask = __LOG_WARNING | __LOG_NOTICE | __LOG_ERROR;
 		memcpy(&chan->formatter, &logformatter_default, sizeof(chan->formatter));
 
-		if (!locked) {
-			AST_RWLIST_WRLOCK(&logchannels);
-		}
 		AST_RWLIST_INSERT_HEAD(&logchannels, chan, list);
 		global_logmask |= chan->logmask;
-		if (!locked) {
-			AST_RWLIST_UNLOCK(&logchannels);
-		}
 
 		return -1;
 	}
@@ -675,9 +663,6 @@
 		}
 	}
 
-	if (!locked) {
-		AST_RWLIST_WRLOCK(&logchannels);
-	}
 	var = ast_variable_browse(cfg, "logfiles");
 	for (; var; var = var->next) {
 		if (!(chan = make_logchannel(var->name, var->value, var->lineno, 0))) {
@@ -695,10 +680,6 @@
 	if (qlog) {
 		fclose(qlog);
 		qlog = NULL;
-	}
-
-	if (!locked) {
-		AST_RWLIST_UNLOCK(&logchannels);
 	}
 
 	ast_config_destroy(cfg);
@@ -1055,7 +1036,7 @@
 
 	filesize_reload_needed = 0;
 
-	init_logger_chain(1 /* locked */, altconf);
+	init_logger_chain(altconf);
 
 	ast_unload_realtime("queue_log");
 	if (logfiles.queue_log) {
@@ -1153,7 +1134,7 @@
 		}
 	}
 
-	init_logger_chain(1 /* locked */, NULL);
+	init_logger_chain(NULL);
 
 	AST_RWLIST_UNLOCK(&logchannels);
 
@@ -1725,7 +1706,9 @@
 	ast_mkdir(ast_config_AST_LOG_DIR, 0777);
 
 	/* create log channels */
-	res = init_logger_chain(0 /* locked */, NULL);
+	AST_RWLIST_WRLOCK(&logchannels);
+	res = init_logger_chain(NULL);
+	AST_RWLIST_UNLOCK(&logchannels);
 	ast_verb_update();
 	logger_initialized = 1;
 	if (res) {

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6cc42117338bf9575650a67bcb78ab1a33d7bad8
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>