Merge "logger: Prevent duplicate dynamic channels from being added." into 13

main/logger.c

@@ -286,6 +286,70 @@ static void make_components(struct logchannel *chan)
 	chan->logmask = logmask;
 }
 
+/*!
+ * \brief create the filename that will be used for a logger channel.
+ *
+ * \param channel The name of the logger channel
+ * \param[out] filename The filename for the logger channel
+ * \param size The size of the filename buffer
+ */
+static void make_filename(const char *channel, char *filename, size_t size)
+{
+	const char *log_dir_prefix = "";
+	const char *log_dir_separator = "";
+
+	*filename = '\0';
+
+	if (!strcasecmp(channel, "console")) {
+		return;
+	}
+
+	if (!strncasecmp(channel, "syslog", 6)) {
+		ast_copy_string(filename, channel, size);
+		return;
+	}
+
+	/* It's a filename */
+
+	if (channel[0] != '/') {
+		log_dir_prefix = ast_config_AST_LOG_DIR;
+		log_dir_separator = "/";
+	}
+
+	if (!ast_strlen_zero(hostname)) {
+		snprintf(filename, size, "%s%s%s.%s",
+			log_dir_prefix, log_dir_separator, channel, hostname);
+	} else {
+		snprintf(filename, size, "%s%s%s",
+			log_dir_prefix, log_dir_separator, channel);
+	}
+}
+
+/*!
+ * \brief Find a particular logger channel by name
+ *
+ * \pre logchannels list is locked
+ *
+ * \param channel The name of the logger channel to find
+ * \retval non-NULL The corresponding logger channel
+ * \retval NULL Unable to find a logger channel with that particular name
+ */
+static struct logchannel *find_logchannel(const char *channel)
+{
+	char filename[PATH_MAX];
+	struct logchannel *chan;
+
+	make_filename(channel, filename, sizeof(filename));
+
+	AST_RWLIST_TRAVERSE(&logchannels, chan, list) {
+		if (!strcmp(chan->filename, filename)) {
+			return chan;
+		}
+	}
+
+	return NULL;
+}
+
 static struct logchannel *make_logchannel(const char *channel, const char *components, int lineno, int dynamic)
 {
 	struct logchannel *chan;
@@ -301,6 +365,8 @@ static struct logchannel *make_logchannel(const char *channel, const char *compo
 	chan->lineno = lineno;
 	chan->dynamic = dynamic;
 
+	make_filename(channel, chan->filename, sizeof(chan->filename));
+
 	if (!strcasecmp(channel, "console")) {
 		chan->type = LOGTYPE_CONSOLE;
 	} else if (!strncasecmp(channel, "syslog", 6)) {
@@ -322,24 +388,7 @@ static struct logchannel *make_logchannel(const char *channel, const char *compo
 		}
 
 		chan->type = LOGTYPE_SYSLOG;
-		ast_copy_string(chan->filename, channel, sizeof(chan->filename));
 	} else {
-		const char *log_dir_prefix = "";
-		const char *log_dir_separator = "";
-
-		if (channel[0] != '/') {
-			log_dir_prefix = ast_config_AST_LOG_DIR;
-			log_dir_separator = "/";
-		}
-
-		if (!ast_strlen_zero(hostname)) {
-			snprintf(chan->filename, sizeof(chan->filename), "%s%s%s.%s",
-				log_dir_prefix, log_dir_separator, channel, hostname);
-		} else {
-			snprintf(chan->filename, sizeof(chan->filename), "%s%s%s",
-				log_dir_prefix, log_dir_separator, channel);
-		}
-
 		if (!(chan->fileptr = fopen(chan->filename, "a"))) {
 			/* Can't do real logging here since we're called with a lock
 			 * so log to any attached consoles */
@@ -936,13 +985,9 @@ int ast_logger_rotate_channel(const char *log_channel)
 {
 	struct logchannel *f;
 	int success = AST_LOGGER_FAILURE;
+	char filename[PATH_MAX];
 
-	struct ast_str *filename = ast_str_create(64);
+	make_filename(log_channel, filename, sizeof(filename));
-	if (!filename) {
-		return AST_LOGGER_ALLOC_ERROR;
-	}
-
-	ast_str_append(&filename, 0, "%s/%s", ast_config_AST_LOG_DIR, log_channel);
 
 	AST_RWLIST_WRLOCK(&logchannels);
 
@@ -957,7 +1002,7 @@ int ast_logger_rotate_channel(const char *log_channel)
 		if (f->fileptr && (f->fileptr != stdout) && (f->fileptr != stderr)) {
 			fclose(f->fileptr);	/* Close file */
 			f->fileptr = NULL;
-			if (strcmp(ast_str_buffer(filename), f->filename) == 0) {
+			if (strcmp(filename, f->filename) == 0) {
 				rotate_file(f->filename);
 				success = AST_LOGGER_SUCCESS;
 			}
@@ -967,7 +1012,6 @@ int ast_logger_rotate_channel(const char *log_channel)
 	init_logger_chain(1 /* locked */, NULL);
 
 	AST_RWLIST_UNLOCK(&logchannels);
-	ast_free(filename);
 
 	return success;
 }
@@ -1098,45 +1142,35 @@ static char *handle_logger_show_channels(struct ast_cli_entry *e, int cmd, struc
 int ast_logger_create_channel(const char *log_channel, const char *components)
 {
 	struct logchannel *chan;
-	struct ast_str *filename = ast_str_create(64);
-	int chan_exists = AST_LOGGER_SUCCESS;
 
 	if (ast_strlen_zero(components)) {
 		return AST_LOGGER_DECLINE;
 	}
 
-	if (!filename) {
+	AST_RWLIST_WRLOCK(&logchannels);
+
+	chan = find_logchannel(log_channel);
+	if (chan) {
+		AST_RWLIST_UNLOCK(&logchannels);
+		return AST_LOGGER_FAILURE;
+	}
+
+	chan = make_logchannel(log_channel, components, 0, 1);
+	if (!chan) {
+		AST_RWLIST_UNLOCK(&logchannels);
 		return AST_LOGGER_ALLOC_ERROR;
 	}
 
-	ast_str_append(&filename, 0, "%s/%s", ast_config_AST_LOG_DIR, log_channel);
+	AST_RWLIST_INSERT_HEAD(&logchannels, chan, list);
+	global_logmask |= chan->logmask;
 
-	AST_RWLIST_WRLOCK(&logchannels);
-
-	AST_RWLIST_TRAVERSE(&logchannels, chan, list) {
-		if (!strcmp(ast_str_buffer(filename), chan->filename)) {
-			chan_exists = AST_LOGGER_FAILURE;
-			break;
-		}
-	}
-
-	if (!chan_exists) {
-		chan = make_logchannel(log_channel, components, 0, 1);
-		if (chan) {
-			AST_RWLIST_INSERT_HEAD(&logchannels, chan, list);
-			global_logmask |= chan->logmask;
-			chan_exists = AST_LOGGER_SUCCESS;
-		}
-	}
 	AST_RWLIST_UNLOCK(&logchannels);
 
-	return chan_exists;
+	return AST_LOGGER_SUCCESS;
 }
 
 static char *handle_logger_add_channel(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
-	struct logchannel *chan;
-
 	switch (cmd) {
 	case CLI_INIT:
 		e->command = "logger add channel";
@@ -1155,57 +1189,34 @@ static char *handle_logger_add_channel(struct ast_cli_entry *e, int cmd, struct
 		return CLI_SHOWUSAGE;
 	}
 
-	AST_RWLIST_WRLOCK(&logchannels);
+	switch (ast_logger_create_channel(a->argv[3], a->argv[4])) {
-	AST_RWLIST_TRAVERSE(&logchannels, chan, list) {
+	case AST_LOGGER_SUCCESS:
-		if (!strcmp(chan->filename, a->argv[3])) {
+		return CLI_SUCCESS;
-			break;
+	case AST_LOGGER_FAILURE:
-		}
-	}
-
-	if (chan) {
-		AST_RWLIST_UNLOCK(&logchannels);
 		ast_cli(a->fd, "Logger channel '%s' already exists\n", a->argv[3]);
 		return CLI_SUCCESS;
+	case AST_LOGGER_DECLINE:
+	case AST_LOGGER_ALLOC_ERROR:
+	default:
+		ast_cli(a->fd, "ERROR: Unable to create log channel '%s'\n", a->argv[3]);
+		return CLI_FAILURE;
 	}
-
-	chan = make_logchannel(a->argv[3], a->argv[4], 0, 1);
-	if (chan) {
-		AST_RWLIST_INSERT_HEAD(&logchannels, chan, list);
-		global_logmask |= chan->logmask;
-		AST_RWLIST_UNLOCK(&logchannels);
-		return CLI_SUCCESS;
-	}
-
-	AST_RWLIST_UNLOCK(&logchannels);
-	ast_cli(a->fd, "ERROR: Unable to create log channel '%s'\n", a->argv[3]);
-
-	return CLI_FAILURE;
 }
 
 int ast_logger_remove_channel(const char *log_channel)
 {
 	struct logchannel *chan;
-	struct ast_str *filename = ast_str_create(64);
-
-	if (!filename) {
-		return AST_LOGGER_ALLOC_ERROR;
-	}
-
-	ast_str_append(&filename, 0, "%s/%s", ast_config_AST_LOG_DIR, log_channel);
 
 	AST_RWLIST_WRLOCK(&logchannels);
-	AST_RWLIST_TRAVERSE_SAFE_BEGIN(&logchannels, chan, list) {
-		if (chan->dynamic && !strcmp(chan->filename, ast_str_buffer(filename))) {
-			AST_RWLIST_REMOVE_CURRENT(list);
-			break;
-		}
-	}
-	AST_RWLIST_TRAVERSE_SAFE_END;
-	AST_RWLIST_UNLOCK(&logchannels);
 
-	if (!chan) {
+	chan = find_logchannel(log_channel);
+	if (chan && chan->dynamic) {
+		AST_RWLIST_REMOVE(&logchannels, chan, list);
+	} else {
+		AST_RWLIST_UNLOCK(&logchannels);
 		return AST_LOGGER_FAILURE;
 	}
+	AST_RWLIST_UNLOCK(&logchannels);
 
 	if (chan->fileptr) {
 		fclose(chan->fileptr);
@@ -1253,30 +1264,17 @@ static char *handle_logger_remove_channel(struct ast_cli_entry *e, int cmd, stru
 		return CLI_SHOWUSAGE;
 	}
 
-	AST_RWLIST_WRLOCK(&logchannels);
+	switch (ast_logger_remove_channel(a->argv[3])) {
-	AST_RWLIST_TRAVERSE_SAFE_BEGIN(&logchannels, chan, list) {
+	case AST_LOGGER_SUCCESS:
-		if (chan->dynamic && !strcmp(chan->filename, a->argv[3])) {
+		ast_cli(a->fd, "Removed dynamic logger channel '%s'\n", a->argv[3]);
-			AST_RWLIST_REMOVE_CURRENT(list);
+		return CLI_SUCCESS;
-			break;
+	case AST_LOGGER_FAILURE:
-		}
-	}
-	AST_RWLIST_TRAVERSE_SAFE_END;
-	AST_RWLIST_UNLOCK(&logchannels);
-
-	if (!chan) {
 		ast_cli(a->fd, "Unable to find dynamic logger channel '%s'\n", a->argv[3]);
 		return CLI_SUCCESS;
+	default:
+		ast_cli(a->fd, "Internal failure attempting to delete dynamic logger channel '%s'\n", a->argv[3]);
+		return CLI_FAILURE;
 	}
-
-	ast_cli(a->fd, "Removed dynamic logger channel '%s'\n", chan->filename);
-	if (chan->fileptr) {
-		fclose(chan->fileptr);
-		chan->fileptr = NULL;
-	}
-	ast_free(chan);
-	chan = NULL;
-
-	return CLI_SUCCESS;
 }
 
 struct verb {