Merge "logger: Prevent duplicate dynamic channels from being added."
authorJoshua Colp <jcolp@digium.com>
Fri, 25 Sep 2015 15:57:40 +0000 (10:57 -0500)
committerGerrit Code Review <gerrit2@gerrit.digium.api>
Fri, 25 Sep 2015 15:57:40 +0000 (10:57 -0500)
main/logger.c

index 12410ca..b02e08e 100644 (file)
@@ -278,6 +278,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;
@@ -293,6 +357,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)) {
@@ -314,24 +380,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 */
@@ -928,13 +977,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);
-       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);
 
@@ -949,7 +994,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;
                        }
@@ -959,7 +1004,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;
 }
@@ -1090,45 +1134,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) {
-               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(&logchannels, chan, list) {
-               if (!strcmp(ast_str_buffer(filename), chan->filename)) {
-                       chan_exists = AST_LOGGER_FAILURE;
-                       break;
-               }
+       chan = find_logchannel(log_channel);
+       if (chan) {
+               AST_RWLIST_UNLOCK(&logchannels);
+               return AST_LOGGER_FAILURE;
        }
 
-       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;
-               }
+       chan = make_logchannel(log_channel, components, 0, 1);
+       if (!chan) {
+               AST_RWLIST_UNLOCK(&logchannels);
+               return AST_LOGGER_ALLOC_ERROR;
        }
+
+       AST_RWLIST_INSERT_HEAD(&logchannels, chan, list);
+       global_logmask |= chan->logmask;
+
        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";
@@ -1147,57 +1181,34 @@ static char *handle_logger_add_channel(struct ast_cli_entry *e, int cmd, struct
                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);
-               ast_cli(a->fd, "Logger channel '%s' already exists\n", a->argv[3]);
+       switch (ast_logger_create_channel(a->argv[3], a->argv[4])) {
+       case AST_LOGGER_SUCCESS:
                return CLI_SUCCESS;
-       }
-
-       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);
+       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;
        }
-
-       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);
@@ -1245,30 +1256,17 @@ static char *handle_logger_remove_channel(struct ast_cli_entry *e, int cmd, stru
                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 {