[asterisk-commits] logger: Prevent duplicate dynamic channels from being added. (asterisk[13])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Sep 25 10:57:34 CDT 2015
Joshua Colp has submitted this change and it was merged.
Change subject: logger: Prevent duplicate dynamic channels from being added.
......................................................................
logger: Prevent duplicate dynamic channels from being added.
There was a problem observed where the "logger add channel" CLI command
would allow for a channel with the same name to be added multiple times.
This would result in each message being written out to the same file
multiple times.
The problem was due to the difference in how logger channel filenames
are stored versus the format they are allowed to be presented when they
are added. For instance, if adding the logger channel "foo" through the
CLI, the result would be a logger channel with the file name
/var/log/asterisk/foo being stored. So when trying to add another "foo"
channel, "foo" would not match "/var/log/asterisk/foo" so we'd happily
add the duplicate channel.
The fix presented here is to introduce two new methods in the logger
code:
* make_filename(): given a logger channel name, this creates the
filename for that logger channel.
* find_logchannel(): given a logger channel name, this calls
make_filename() and then traverses the list of logchannels in order
to find a match.
This change has made use of make_filename() and find_logchannel()
throughout to more consistently behave.
ASTERISK-25305 #close
Reported by Mark Michelson
Change-Id: I892d52954d6007d8bc453c3cbdd9235dec9c4a36
---
M main/logger.c
1 file changed, 106 insertions(+), 108 deletions(-)
Approvals:
Kevin Harwell: Looks good to me, but someone else must approve
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/main/logger.c b/main/logger.c
index eccf556..fb9e8ed 100644
--- a/main/logger.c
+++ b/main/logger.c
@@ -286,6 +286,70 @@
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;
@@ -300,6 +364,8 @@
strcpy(chan->components, components);
chan->lineno = lineno;
chan->dynamic = dynamic;
+
+ make_filename(channel, chan->filename, sizeof(chan->filename));
if (!strcasecmp(channel, "console")) {
chan->type = LOGTYPE_CONSOLE;
@@ -322,24 +388,7 @@
}
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 @@
{
struct logchannel *f;
int success = AST_LOGGER_FAILURE;
+ char filename[PATH_MAX];
- 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);
+ make_filename(log_channel, filename, sizeof(filename));
AST_RWLIST_WRLOCK(&logchannels);
@@ -957,7 +1002,7 @@
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 @@
init_logger_chain(1 /* locked */, NULL);
AST_RWLIST_UNLOCK(&logchannels);
- ast_free(filename);
return success;
}
@@ -1098,45 +1142,35 @@
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 @@
return CLI_SHOWUSAGE;
}
- AST_RWLIST_WRLOCK(&logchannels);
- AST_RWLIST_TRAVERSE(&logchannels, chan, list) {
- if (!strcmp(chan->filename, a->argv[3])) {
- break;
- }
- }
-
- if (chan) {
- AST_RWLIST_UNLOCK(&logchannels);
+ switch (ast_logger_create_channel(a->argv[3], a->argv[4])) {
+ case AST_LOGGER_SUCCESS:
+ return CLI_SUCCESS;
+ case AST_LOGGER_FAILURE:
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 @@
return CLI_SHOWUSAGE;
}
- AST_RWLIST_WRLOCK(&logchannels);
- AST_RWLIST_TRAVERSE_SAFE_BEGIN(&logchannels, chan, list) {
- if (chan->dynamic && !strcmp(chan->filename, a->argv[3])) {
- AST_RWLIST_REMOVE_CURRENT(list);
- break;
- }
- }
- AST_RWLIST_TRAVERSE_SAFE_END;
- AST_RWLIST_UNLOCK(&logchannels);
-
- if (!chan) {
+ switch (ast_logger_remove_channel(a->argv[3])) {
+ case AST_LOGGER_SUCCESS:
+ ast_cli(a->fd, "Removed dynamic logger channel '%s'\n", a->argv[3]);
+ return CLI_SUCCESS;
+ case AST_LOGGER_FAILURE:
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 {
--
To view, visit https://gerrit.asterisk.org/1303
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I892d52954d6007d8bc453c3cbdd9235dec9c4a36
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-commits
mailing list