From 7b50bbf6c16f72b6bcd47cc5f827951d97e33254 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Wed, 5 Dec 2012 00:49:53 +0000 Subject: [PATCH] confbridge: Fix several small issues. * Made func_confbridge_helper() allow an empty value when setting options. You previously could not Set(CONFBRIDGE(user,pin)=) and clear the configured pin from the dialplan. * Made func_confbridge_helper() handle its datastore better if multiple threads attempt to set the first CONFBRIDGE option value on the channel. * Made the func_confbridge_helper() only output one diagnostic message concerning the option. * Made the bridge video_mode able to repeatedly change in the config file and CONFBRIDGE dialplan function. The video_mode option values are an enum and not independent of each other. * Made handle_cli_confbridge_show_bridge_profile() better handle the video_mode option. * Simplified datastore handling code in conf_find_user_profile() and conf_find_bridge_profile(). * Made parse_bridge(), parse_user(), and parse_menu() use var->file instead of CONFBRIDGE_CONFIG because the var could have been from an include file. (closes issue ASTERISK-20655) Reported by: Birger "WIMPy" Harzenetter git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/10@377227 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- apps/confbridge/conf_config_parser.c | 131 ++++++++++++++++----------- 1 file changed, 79 insertions(+), 52 deletions(-) diff --git a/apps/confbridge/conf_config_parser.c b/apps/confbridge/conf_config_parser.c index 234b86a3dc..9fe32ab127 100644 --- a/apps/confbridge/conf_config_parser.c +++ b/apps/confbridge/conf_config_parser.c @@ -286,7 +286,6 @@ static int set_bridge_option(const char *name, const char *value, struct bridge_ case 80: break; default: - ast_log(LOG_WARNING, "invalid mixing interval %u\n", b_profile->mix_interval); b_profile->mix_interval = 0; return -1; } @@ -294,11 +293,30 @@ static int set_bridge_option(const char *name, const char *value, struct bridge_ ast_set2_flag(b_profile, ast_true(value), BRIDGE_OPT_RECORD_CONFERENCE); } else if (!strcasecmp(name, "video_mode")) { if (!strcasecmp(value, "first_marked")) { - ast_set_flag(b_profile, BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED); + ast_set_flags_to(b_profile, + BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED + | BRIDGE_OPT_VIDEO_SRC_LAST_MARKED + | BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER, + BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED); } else if (!strcasecmp(value, "last_marked")) { - ast_set_flag(b_profile, BRIDGE_OPT_VIDEO_SRC_LAST_MARKED); + ast_set_flags_to(b_profile, + BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED + | BRIDGE_OPT_VIDEO_SRC_LAST_MARKED + | BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER, + BRIDGE_OPT_VIDEO_SRC_LAST_MARKED); } else if (!strcasecmp(value, "follow_talker")) { - ast_set_flag(b_profile, BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER); + ast_set_flags_to(b_profile, + BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED + | BRIDGE_OPT_VIDEO_SRC_LAST_MARKED + | BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER, + BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER); + } else if (!strcasecmp(value, "none")) { + ast_clear_flag(b_profile, + BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED + | BRIDGE_OPT_VIDEO_SRC_LAST_MARKED + | BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER); + } else { + return -1; } } else if (!strcasecmp(name, "max_members")) { if (sscanf(value, "%30u", &b_profile->max_members) != 1) { @@ -346,7 +364,7 @@ static int set_bridge_option(const char *name, const char *value, struct bridge_ ast_string_field_set(sounds, leave, tmp->sounds->leave); ao2_ref(tmp->sounds, -1); /* sounds struct copied over to it from the template by reference only. */ - ao2_ref(oldsounds,-1); /* original sounds struct we don't need anymore */ + ao2_ref(oldsounds, -1); /* original sounds struct we don't need anymore */ tmp->sounds = sounds; /* the new sounds struct that is a deep copy of the one from the template. */ } else { return -1; @@ -374,17 +392,16 @@ static const struct ast_datastore_info confbridge_datastore = { }; int func_confbridge_helper(struct ast_channel *chan, const char *cmd, char *data, const char *value) { - struct ast_datastore *datastore = NULL; - struct func_confbridge_data *b_data = NULL; - char *parse = NULL; - int new = 0; + struct ast_datastore *datastore; + struct func_confbridge_data *b_data; + char *parse; AST_DECLARE_APP_ARGS(args, AST_APP_ARG(type); AST_APP_ARG(option); ); /* parse all the required arguments and make sure they exist. */ - if (ast_strlen_zero(data) || ast_strlen_zero(value)) { + if (ast_strlen_zero(data)) { return -1; } parse = ast_strdupa(data); @@ -394,50 +411,51 @@ int func_confbridge_helper(struct ast_channel *chan, const char *cmd, char *data } ast_channel_lock(chan); - if (!(datastore = ast_channel_datastore_find(chan, &confbridge_datastore, NULL))) { - ast_channel_unlock(chan); - - if (!(datastore = ast_datastore_alloc(&confbridge_datastore, NULL))) { + datastore = ast_channel_datastore_find(chan, &confbridge_datastore, NULL); + if (!datastore) { + datastore = ast_datastore_alloc(&confbridge_datastore, NULL); + if (!datastore) { + ast_channel_unlock(chan); return 0; } - if (!(b_data = ast_calloc(1, sizeof(*b_data)))) { + b_data = ast_calloc(1, sizeof(*b_data)); + if (!b_data) { + ast_channel_unlock(chan); ast_datastore_free(datastore); return 0; } - if (!(b_data->b_profile.sounds = bridge_profile_sounds_alloc())) { + b_data->b_profile.sounds = bridge_profile_sounds_alloc(); + if (!b_data->b_profile.sounds) { + ast_channel_unlock(chan); ast_datastore_free(datastore); ast_free(b_data); return 0; } datastore->data = b_data; - new = 1; + ast_channel_datastore_add(chan, datastore); } else { - ast_channel_unlock(chan); b_data = datastore->data; } + ast_channel_unlock(chan); /* SET(CONFBRIDGE(type,option)=value) */ - if (!strcasecmp(args.type, "bridge") && !set_bridge_option(args.option, value, &b_data->b_profile)) { - b_data->b_usable = 1; - } else if (!strcasecmp(args.type, "user") && !set_user_option(args.option, value, &b_data->u_profile)) { - b_data->u_usable = 1; - } else { - ast_log(LOG_WARNING, "Profile type \"%s\" can not be set in CONFBRIDGE function with option \"%s\" and value \"%s\"\n", - args.type, args.option, value); - goto cleanup_error; + if (!value) { + value = ""; } - if (new) { - ast_channel_lock(chan); - ast_channel_datastore_add(chan, datastore); - ast_channel_unlock(chan); + if (!strcasecmp(args.type, "bridge")) { + if (!set_bridge_option(args.option, value, &b_data->b_profile)) { + b_data->b_usable = 1; + return 0; + } + } else if (!strcasecmp(args.type, "user")) { + if (!set_user_option(args.option, value, &b_data->u_profile)) { + b_data->u_usable = 1; + return 0; + } } - return 0; -cleanup_error: - ast_log(LOG_ERROR, "Invalid argument provided to the %s function\n", cmd); - if (new) { - ast_datastore_free(datastore); - } + ast_log(LOG_WARNING, "%s(%s,%s) cannot be set to '%s'. Invalid type, option, or value.\n", + cmd, args.type, args.option, value); return -1; } @@ -486,7 +504,7 @@ static int parse_bridge(const char *cat, struct ast_config *cfg) continue; } else if (set_bridge_option(var->name, var->value, b_profile)) { ast_log(LOG_WARNING, "Invalid: '%s' at line %d of %s is not supported.\n", - var->name, var->lineno, CONFBRIDGE_CONFIG); + var->name, var->lineno, var->file); } } ao2_unlock(b_profile); @@ -524,7 +542,7 @@ static int parse_user(const char *cat, struct ast_config *cfg) continue; } else if (set_user_option(var->name, var->value, u_profile)) { ast_log(LOG_WARNING, "Invalid option '%s' at line %d of %s is not supported.\n", - var->name, var->lineno, CONFBRIDGE_CONFIG); + var->name, var->lineno, var->file); } } ao2_unlock(u_profile); @@ -755,7 +773,7 @@ static int parse_menu(const char *cat, struct ast_config *cfg) continue; } else if (add_menu_entry(menu, var->name, var->value)) { ast_log(LOG_WARNING, "Unknown option '%s' at line %d of %s is not supported.\n", - var->name, var->lineno, CONFBRIDGE_CONFIG); + var->name, var->lineno, var->file); } } ao2_unlock(menu); @@ -1009,14 +1027,25 @@ static char *handle_cli_confbridge_show_bridge_profile(struct ast_cli_entry *e, ast_cli(a->fd,"Max Members: No Limit\n"); } - if (b_profile.flags & BRIDGE_OPT_VIDEO_SRC_LAST_MARKED) { + switch (b_profile.flags + & (BRIDGE_OPT_VIDEO_SRC_LAST_MARKED | BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED + | BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER)) { + case BRIDGE_OPT_VIDEO_SRC_LAST_MARKED: ast_cli(a->fd, "Video Mode: last_marked\n"); - } else if (b_profile.flags & BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED) { + break; + case BRIDGE_OPT_VIDEO_SRC_FIRST_MARKED: ast_cli(a->fd, "Video Mode: first_marked\n"); - } else if (b_profile.flags & BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER) { + break; + case BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER: ast_cli(a->fd, "Video Mode: follow_talker\n"); - } else { + break; + case 0: ast_cli(a->fd, "Video Mode: no video\n"); + break; + default: + /* Opps. We have more than one video mode flag set. */ + ast_assert(0); + break; } ast_cli(a->fd,"sound_join: %s\n", conf_get_sound(CONF_SOUND_JOIN, b_profile.sounds)); @@ -1229,7 +1258,7 @@ static int conf_parse_init(void) return 0; } -void conf_destroy_config() +void conf_destroy_config(void) { if (user_profiles) { ao2_ref(user_profiles, -1); @@ -1316,15 +1345,14 @@ const struct user_profile *conf_find_user_profile(struct ast_channel *chan, cons if (chan) { ast_channel_lock(chan); - if ((datastore = ast_channel_datastore_find(chan, &confbridge_datastore, NULL))) { - ast_channel_unlock(chan); + datastore = ast_channel_datastore_find(chan, &confbridge_datastore, NULL); + ast_channel_unlock(chan); + if (datastore) { b_data = datastore->data; if (b_data->u_usable) { conf_user_profile_copy(result, &b_data->u_profile); return result; } - } else { - ast_channel_unlock(chan); } } @@ -1367,15 +1395,14 @@ const struct bridge_profile *conf_find_bridge_profile(struct ast_channel *chan, if (chan) { ast_channel_lock(chan); - if ((datastore = ast_channel_datastore_find(chan, &confbridge_datastore, NULL))) { - ast_channel_unlock(chan); + datastore = ast_channel_datastore_find(chan, &confbridge_datastore, NULL); + ast_channel_unlock(chan); + if (datastore) { b_data = datastore->data; if (b_data->b_usable) { conf_bridge_profile_copy(result, &b_data->b_profile); return result; } - } else { - ast_channel_unlock(chan); } } if (ast_strlen_zero(bridge_profile_name)) { -- 2.47.2