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().

(closes issue ASTERISK-20655)
Reported by: Birger "WIMPy" Harzenetter
........

Merged revisions 377227 from http://svn.asterisk.org/svn/asterisk/branches/10
........

Merged revisions 377228 from http://svn.asterisk.org/svn/asterisk/branches/11


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@377229 65c4cc65-6c06-0410-ace0-fbb531ad65f3

1 files changed, 74 insertions, 50 deletions

diff --git a/apps/confbridge/conf_config_parser.c b/apps/confbridge/conf_config_parser.c
index 128691458..39bf298b1 100644
--- a/apps/confbridge/conf_config_parser.c
+++ b/apps/confbridge/conf_config_parser.c
@@ -335,10 +335,9 @@ 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;
 	struct ast_variable tmpvar = { 0, };
 	AST_DECLARE_APP_ARGS(args,
 		AST_APP_ARG(type);
@@ -346,7 +345,7 @@ int func_confbridge_helper(struct ast_channel *chan, const char *cmd, char *data
 	);
 
 	/* 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);
@@ -356,53 +355,54 @@ 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 (!value) {
+		value = "";
+	}
 	tmpvar.name = args.option;
 	tmpvar.value = value;
 	tmpvar.file = "CONFBRIDGE";
-	/* SET(CONFBRIDGE(type,option)=value) */
-	if (!strcasecmp(args.type, "bridge") && !aco_process_var(&bridge_type, "dialplan", &tmpvar, &b_data->b_profile)) {
-		b_data->b_usable = 1;
-	} else if (!strcasecmp(args.type, "user") && !aco_process_var(&user_type, "dialplan", &tmpvar, &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 (new) {
-		ast_channel_lock(chan);
-		ast_channel_datastore_add(chan, datastore);
-		ast_channel_unlock(chan);
+	if (!strcasecmp(args.type, "bridge")) {
+		if (!aco_process_var(&bridge_type, "dialplan", &tmpvar, &b_data->b_profile)) {
+			b_data->b_usable = 1;
+			return 0;
+		}
+	} else if (!strcasecmp(args.type, "user")) {
+		if (!aco_process_var(&user_type, "dialplan", &tmpvar, &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;
 }
 
@@ -878,14 +878,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));
@@ -1178,13 +1189,28 @@ static int video_mode_handler(const struct aco_option *opt, struct ast_variable
 		return -1;
 	}
 	if (!strcasecmp(var->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(var->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(var->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(var->value, "none")) {
-		return 0;
+		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;
 	}
@@ -1235,7 +1261,7 @@ static int bridge_template_handler(const struct aco_option *opt, struct ast_vari
 	ast_string_field_set(sounds, participantsunmuted, b_profile->sounds->participantsunmuted);
 
 	ao2_ref(b_profile->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 */
 	b_profile->sounds = sounds;     /* the new sounds struct that is a deep copy of the one from the template. */
 
 	return 0;
@@ -1341,15 +1367,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);
 		}
 	}
 
@@ -1396,15 +1421,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)) {