[Asterisk-code-review] asterisk: Audit locking of channel when manipulating flags. (asterisk[master])

Joshua Colp asteriskteam at digium.com
Mon May 15 11:04:29 CDT 2017


Joshua Colp has uploaded a new change for review. ( https://gerrit.asterisk.org/5625 )

Change subject: asterisk: Audit locking of channel when manipulating flags.
......................................................................

asterisk: Audit locking of channel when manipulating flags.

When manipulating flags on a channel the channel has to be
locked to guarantee that nothing else is also manipulating
the flags. This change introduces locking where necessary to
guarantee this. It also adds helper functions that manipulate
channel flags and lock to reduce repeated code.

ASTERISK-26789

Change-Id: I489280662dba0f4c50981bfc5b5a7073fef2db10
---
M apps/app_chanspy.c
M apps/app_dial.c
M apps/app_disa.c
M apps/app_dumpchan.c
M apps/app_externalivr.c
M include/asterisk/channel.h
M main/autoservice.c
M main/bridge.c
M main/bridge_after.c
M main/bridge_channel.c
M main/channel.c
M main/file.c
M main/manager.c
M main/pbx.c
M main/pbx_builtins.c
M res/res_agi.c
M res/res_musiconhold.c
17 files changed, 134 insertions(+), 77 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/25/5625/1

diff --git a/apps/app_chanspy.c b/apps/app_chanspy.c
index 4814f4c..354b9ea 100644
--- a/apps/app_chanspy.c
+++ b/apps/app_chanspy.c
@@ -693,9 +693,7 @@
 		}
 	}
 
-	ast_channel_lock(chan);
-	ast_set_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
-	ast_channel_unlock(chan);
+	ast_channel_set_flag(chan, AST_FLAG_END_DTMF_ONLY);
 
 	csth.volfactor = *volfactor;
 
@@ -825,9 +823,7 @@
 	else
 		ast_deactivate_generator(chan);
 
-	ast_channel_lock(chan);
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
-	ast_channel_unlock(chan);
+	ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 
 	if (ast_test_flag(flags, OPTION_WHISPER | OPTION_BARGE | OPTION_DTMF_SWITCH_MODES)) {
 		ast_audiohook_lock(&csth.whisper_audiohook);
@@ -921,7 +917,7 @@
 	if (ast_channel_state(chan) != AST_STATE_UP)
 		ast_answer(chan);
 
-	ast_set_flag(ast_channel_flags(chan), AST_FLAG_SPYING); /* so nobody can spy on us while we are spying */
+	ast_channel_set_flag(chan, AST_FLAG_SPYING);
 
 	waitms = 100;
 
@@ -934,7 +930,7 @@
 			if (!res)
 				res = ast_waitstream(chan, "");
 			else if (res < 0) {
-				ast_clear_flag(ast_channel_flags(chan), AST_FLAG_SPYING);
+				ast_channel_clear_flag(chan, AST_FLAG_SPYING);
 				break;
 			}
 			if (!ast_strlen_zero(exitcontext)) {
@@ -977,7 +973,7 @@
 		res = ast_waitfordigit(chan, waitms);
 		if (res < 0) {
 			iter = ast_channel_iterator_destroy(iter);
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_SPYING);
+			ast_channel_clear_flag(chan, AST_FLAG_SPYING);
 			break;
 		}
 		if (!ast_strlen_zero(exitcontext)) {
@@ -1196,7 +1192,7 @@
 	}
 exit:
 
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_SPYING);
+	ast_channel_clear_flag(chan, AST_FLAG_SPYING);
 
 	ast_channel_setoption(chan, AST_OPTION_TXGAIN, &zero_volume, sizeof(zero_volume), 0);
 
diff --git a/apps/app_dial.c b/apps/app_dial.c
index 79e2a9b..1aa4c32 100644
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -2855,7 +2855,7 @@
 				ast_log(LOG_ERROR, "error streaming file '%s' to callee\n", opt_args[OPT_ARG_ANNOUNCE]);
 			}
 
-			ast_set_flag(ast_channel_flags(peer), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_set_flag(peer, AST_FLAG_END_DTMF_ONLY);
 			while (ast_channel_stream(peer)) {
 				int ms;
 
@@ -2919,13 +2919,13 @@
 				}
 				ast_sched_runq(ast_channel_sched(peer));
 			}
-			ast_clear_flag(ast_channel_flags(peer), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(peer, AST_FLAG_END_DTMF_ONLY);
 		}
 
 		if (chan && peer && ast_test_flag64(&opts, OPT_GOTO) && !ast_strlen_zero(opt_args[OPT_ARG_GOTO])) {
 			/* chan and peer are going into the PBX; as such neither are considered
 			 * outgoing channels any longer */
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+			ast_channel_clear_flag(chan, AST_FLAG_OUTGOING);
 
 			ast_replace_subargument_delimiter(opt_args[OPT_ARG_GOTO]);
 			ast_parseable_goto(chan, opt_args[OPT_ARG_GOTO]);
@@ -3295,7 +3295,7 @@
 		int continue_exec;
 
 		ast_channel_data_set(chan, "Retrying");
-		if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_MOH))
+		if (ast_channel_test_flag(chan, AST_FLAG_MOH))
 			ast_moh_stop(chan);
 
 		res = dial_exec_full(chan, args.dialdata, &peerflags, &continue_exec);
@@ -3312,7 +3312,7 @@
 						ast_log(LOG_WARNING, "Announce file \"%s\" specified in Retrydial does not exist\n", args.announce);
 				}
 				if (!res && sleepms) {
-					if (!ast_test_flag(ast_channel_flags(chan), AST_FLAG_MOH))
+					if (!ast_channel_test_flag(chan, AST_FLAG_MOH))
 						ast_moh_start(chan, NULL, NULL);
 					res = ast_waitfordigit(chan, sleepms);
 				}
@@ -3325,7 +3325,7 @@
 						ast_log(LOG_WARNING, "Announce file \"%s\" specified in Retrydial does not exist\n", args.announce);
 				}
 				if (sleepms) {
-					if (!ast_test_flag(ast_channel_flags(chan), AST_FLAG_MOH))
+					if (!ast_channel_test_flag(chan, AST_FLAG_MOH))
 						ast_moh_start(chan, NULL, NULL);
 					if (!res)
 						res = ast_waitfordigit(chan, sleepms);
@@ -3348,7 +3348,7 @@
 	else if (res == 1)
 		res = 0;
 
-	if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_MOH))
+	if (ast_channel_test_flag(chan, AST_FLAG_MOH))
 		ast_moh_stop(chan);
  done:
 	return res;
diff --git a/apps/app_disa.c b/apps/app_disa.c
index 8dc61ff..cceb554 100644
--- a/apps/app_disa.c
+++ b/apps/app_disa.c
@@ -208,7 +208,7 @@
 
 	play_dialtone(chan, args.mailbox);
 
-	ast_set_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_set_flag(chan, AST_FLAG_END_DTMF_ONLY);
 
 	for (;;) {
 		  /* if outa time, give em reorder */
@@ -224,7 +224,7 @@
 		}
 
 		if (!(f = ast_read(chan))) {
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 			return -1;
 		}
 
@@ -232,7 +232,7 @@
 			if (f->data.uint32)
 				ast_channel_hangupcause_set(chan, f->data.uint32);
 			ast_frfree(f);
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 			return -1;
 		}
 
@@ -261,7 +261,7 @@
 						fp = fopen(args.passcode,"r");
 						if (!fp) {
 							ast_log(LOG_WARNING,"DISA password file %s not found on chan %s\n",args.passcode,ast_channel_name(chan));
-							ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+							ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 							return -1;
 						}
 						pwline[0] = 0;
@@ -357,7 +357,7 @@
 		}
 	}
 
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 
 	if (k == 3) {
 		int recheck = 0;
diff --git a/apps/app_dumpchan.c b/apps/app_dumpchan.c
index 0789ce0..bec7788 100644
--- a/apps/app_dumpchan.c
+++ b/apps/app_dumpchan.c
@@ -88,8 +88,6 @@
 
 	ast_channel_lock(c);
 	bridge = ast_channel_get_bridge(c);
-	ast_channel_unlock(c);
-
 	snprintf(buf,size,
 		"Name=               %s\n"
 		"Type=               %s\n"
@@ -166,7 +164,7 @@
 		ast_channel_appl(c) ? ast_channel_appl(c) : "(N/A)",
 		ast_channel_data(c) ? S_OR(ast_channel_data(c), "(Empty)") : "(None)",
 		(ast_test_flag(ast_channel_flags(c), AST_FLAG_BLOCKING) ? ast_channel_blockproc(c) : "(Not Blocking)"));
-
+	ast_channel_unlock(c);
 	ao2_cleanup(bridge);
 	return 0;
 }
diff --git a/apps/app_externalivr.c b/apps/app_externalivr.c
index c2224b4..c456779 100644
--- a/apps/app_externalivr.c
+++ b/apps/app_externalivr.c
@@ -642,9 +642,9 @@
 	waitfds[0] = ast_iostream_get_fd(eivr_commands);
 	waitfds[1] = eivr_errors ? ast_iostream_get_fd(eivr_errors) : -1;
 
- 	while (1) {
- 		if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE)) {
- 			ast_chan_log(LOG_ERROR, chan, "Is a zombie\n");
+	while (1) {
+		if (ast_channel_test_flag(chan, AST_FLAG_ZOMBIE)) {
+			ast_chan_log(LOG_ERROR, chan, "Is a zombie\n");
  			break;
  		}
  		if (!hangup_info_sent && !(ast_test_flag(&flags, run_dead)) && ast_check_hangup(chan)) {
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index fd339d9..bafb247 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4437,6 +4437,37 @@
 struct ast_flags *ast_channel_flags(struct ast_channel *chan);
 
 /*!
+ * \since 13.17.0
+ * \brief Set a flag on a channel
+ *
+ * \param chan The channel to set the flag on
+ * \param flag The flag to set
+ */
+
+void ast_channel_set_flag(struct ast_channel *chan, unsigned int flag);
+
+/*!
+ * \since 13.17.0
+ * \param Test a flag on a channel
+ *
+ * \param chan The channel to test the flag on
+ * \param flag The flag to test
+ *
+ * \retval 0 flag not set
+ * \retval 1 flag set
+ */
+unsigned int ast_channel_test_flag(struct ast_channel *chan, unsigned int flag);
+
+/*!
+ * \since 13.17.0
+ * \param Clear a flag on a channel
+ *
+ * \param chan The channel to clear the flag from
+ * \param flag The flag to clear
+ */
+void ast_channel_clear_flag(struct ast_channel *chan, unsigned int flag);
+
+/*!
  * \since 12.4.0
  * \brief Return whether or not any manager variables have been set
  *
diff --git a/main/autoservice.c b/main/autoservice.c
index 11c9eab..d1a0156 100644
--- a/main/autoservice.c
+++ b/main/autoservice.c
@@ -295,11 +295,11 @@
 		res = 0;
 	}
 
+	ast_channel_lock(chan);
 	if (!as->orig_end_dtmf_flag) {
 		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
 	}
 
-	ast_channel_lock(chan);
 	while ((f = AST_LIST_REMOVE_HEAD(&as->deferred_frames, frame_list))) {
 		if (!((1 << f->frametype) & as->ignore_frame_types)) {
 			ast_queue_frame_head(chan, f);
diff --git a/main/bridge.c b/main/bridge.c
index 9d9a311..b7dcaab 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -382,8 +382,8 @@
 	if (!bridge_channel ||
 		!(bridge->technology->capabilities & (AST_BRIDGE_CAPABILITY_1TO1MIX | AST_BRIDGE_CAPABILITY_NATIVE)) ||
 		!(peer = ast_bridge_channel_peer(bridge_channel)) ||
-		ast_test_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_ZOMBIE) ||
-		ast_test_flag(ast_channel_flags(peer->chan), AST_FLAG_ZOMBIE) ||
+		ast_channel_test_flag(bridge_channel->chan, AST_FLAG_ZOMBIE) ||
+		ast_channel_test_flag(peer->chan, AST_FLAG_ZOMBIE) ||
 		ast_check_hangup_locked(bridge_channel->chan) ||
 		ast_check_hangup_locked(peer->chan)) {
 		return;
@@ -2611,7 +2611,7 @@
 	if (!AST_LIST_EMPTY(ast_channel_readq(chan))) {
 		return NULL;
 	}
-	if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_EMULATE_DTMF)) {
+	if (ast_channel_test_flag(chan, AST_FLAG_EMULATE_DTMF)) {
 		return NULL;
 	}
 	if (ast_channel_has_audio_frame_or_monitor(chan)) {
diff --git a/main/bridge_after.c b/main/bridge_after.c
index d649717..f0be3a0 100644
--- a/main/bridge_after.c
+++ b/main/bridge_after.c
@@ -482,7 +482,7 @@
 		}
 	} else if (!ast_check_hangup(chan)) {
 		/* Clear the outgoing flag */
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+		ast_channel_clear_flag(chan, AST_FLAG_OUTGOING);
 
 		if (after_bridge->specific) {
 			goto_failed = ast_explicit_goto(chan, after_bridge->context,
@@ -517,7 +517,7 @@
 				after_bridge->exten, after_bridge->priority + 1);
 		}
 		if (!goto_failed) {
-			if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP)) {
+			if (ast_channel_test_flag(chan, AST_FLAG_IN_AUTOLOOP)) {
 				ast_channel_priority_set(chan, ast_channel_priority(chan) + 1);
 			}
 
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 4f166ff..b0eae5d 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -1212,7 +1212,7 @@
 	 * It may be necessary to resume music on hold after we finish
 	 * playing the announcment.
 	 */
-	if (ast_test_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_MOH)) {
+	if (ast_channel_test_flag(bridge_channel->chan, AST_FLAG_MOH)) {
 		const char *latest_musicclass;
 
 		ast_channel_lock(bridge_channel->chan);
@@ -2097,11 +2097,11 @@
 	/* If we are not going to be hung up after leaving a bridge, and we were an
 	 * outgoing channel, clear the outgoing flag.
 	 */
-	if (ast_test_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_OUTGOING)
+	if (ast_channel_test_flag(bridge_channel->chan, AST_FLAG_OUTGOING)
 	    && (ast_channel_is_leaving_bridge(bridge_channel->chan)
 	        || bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT)) {
 		ast_debug(2, "Channel %s will survive this bridge; clearing outgoing (dialed) flag\n", ast_channel_name(bridge_channel->chan));
-		ast_clear_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_OUTGOING);
+		ast_channel_clear_flag(bridge_channel->chan, AST_FLAG_OUTGOING);
 	}
 
 	bridge->reconfigured = 1;
@@ -2212,14 +2212,14 @@
 	chan = bridge_channel->chan;
 	switch (fr->subclass.integer) {
 	case AST_CONTROL_REDIRECTING:
-		is_caller = !ast_test_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+		is_caller = !ast_channel_test_flag(chan, AST_FLAG_OUTGOING);
 		if (ast_channel_redirecting_sub(NULL, chan, fr, 1) &&
 			ast_channel_redirecting_macro(NULL, chan, fr, is_caller, 1)) {
 			ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
 		}
 		break;
 	case AST_CONTROL_CONNECTED_LINE:
-		is_caller = !ast_test_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+		is_caller = !ast_channel_test_flag(chan, AST_FLAG_OUTGOING);
 		if (ast_channel_connected_line_sub(NULL, chan, fr, 1) &&
 			ast_channel_connected_line_macro(NULL, chan, fr, is_caller, 1)) {
 			ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
@@ -2839,7 +2839,7 @@
 	 * Must be done while "still in the bridge" for ast_async_goto()
 	 * to work right.
 	 */
-	while (ast_test_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT)) {
+	while (ast_channel_test_flag(bridge_channel->chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT)) {
 		sched_yield();
 	}
 	ast_channel_lock(bridge_channel->chan);
diff --git a/main/channel.c b/main/channel.c
index 099e6f6..feda5bb 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1288,8 +1288,10 @@
 	int pre = 0;
 
 	if (chan) {
+		ast_channel_lock(chan);
 		pre = ast_test_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF);
 		ast_set_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF);
+		ast_channel_unlock(chan);
 	}
 	return pre;
 }
@@ -1297,8 +1299,9 @@
 /*! \brief Unset defer DTMF flag on channel */
 void ast_channel_undefer_dtmf(struct ast_channel *chan)
 {
-	if (chan)
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF);
+	if (chan) {
+		ast_channel_clear_flag(chan, AST_FLAG_DEFER_DTMF);
+	}
 }
 
 struct ast_channel *ast_channel_callback(ao2_callback_data_fn *cb_fn, void *arg,
@@ -3219,11 +3222,11 @@
 	int ms;
 
 	/* Stop if we're a zombie or need a soft hangup */
-	if (ast_test_flag(ast_channel_flags(c), AST_FLAG_ZOMBIE) || ast_check_hangup(c))
+	if (ast_channel_test_flag(c, AST_FLAG_ZOMBIE) || ast_check_hangup(c))
 		return -1;
 
 	/* Only look for the end of DTMF, don't bother with the beginning and don't emulate things */
-	ast_set_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_set_flag(c, AST_FLAG_END_DTMF_ONLY);
 
 	/* Wait for a digit, no more than timeout_ms milliseconds total.
 	 * Or, wait indefinitely if timeout_ms is <0.
@@ -3242,12 +3245,12 @@
 			if (errno == 0 || errno == EINTR)
 				continue;
 			ast_log(LOG_WARNING, "Wait failed (%s)\n", strerror(errno));
-			ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 			return -1;
 		} else if (outfd > -1) {
 			/* The FD we were watching has something waiting */
 			ast_log(LOG_WARNING, "The FD we were waiting for has something waiting. Waitfordigit returning numeric 1\n");
-			ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 			return 1;
 		} else if (rchan) {
 			int res;
@@ -3261,13 +3264,13 @@
 			case AST_FRAME_DTMF_END:
 				res = f->subclass.integer;
 				ast_frfree(f);
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return res;
 			case AST_FRAME_CONTROL:
 				switch (f->subclass.integer) {
 				case AST_CONTROL_HANGUP:
 					ast_frfree(f);
-					ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+					ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 					return -1;
 				case AST_CONTROL_STREAM_STOP:
 				case AST_CONTROL_STREAM_SUSPEND:
@@ -3278,7 +3281,7 @@
 					 * that perform stream control will handle this. */
 					res = f->subclass.integer;
 					ast_frfree(f);
-					ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+					ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 					return res;
 				case AST_CONTROL_PVT_CAUSE_CODE:
 				case AST_CONTROL_RINGING:
@@ -3313,7 +3316,7 @@
 		}
 	}
 
-	ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 
 	return 0; /* Time is up */
 }
@@ -5630,7 +5633,7 @@
  */
 static void call_forward_inherit(struct ast_channel *new_chan, struct ast_channel *parent, struct ast_channel *orig)
 {
-	if (!ast_test_flag(ast_channel_flags(parent), AST_FLAG_ZOMBIE) && !ast_check_hangup(parent)) {
+	if (!ast_channel_test_flag(parent, AST_FLAG_ZOMBIE) && !ast_check_hangup(parent)) {
 		struct ast_party_redirecting redirecting;
 
 		/*
@@ -5709,9 +5712,9 @@
 	} else if (caller) { /* no outgoing helper so use caller if available */
 		call_forward_inherit(new_chan, caller, orig);
 	}
-	ast_set_flag(ast_channel_flags(new_chan), AST_FLAG_ORIGINATED);
 
 	ast_channel_lock_both(orig, new_chan);
+	ast_channel_set_flag(new_chan, AST_FLAG_ORIGINATED);
 	pbx_builtin_setvar_helper(new_chan, "FORWARDERNAME", forwarder);
 	ast_party_connected_line_copy(ast_channel_connected(new_chan), ast_channel_connected(orig));
 	ast_party_redirecting_copy(ast_channel_redirecting(new_chan), ast_channel_redirecting(orig));
@@ -6304,7 +6307,7 @@
 	struct ast_silence_generator *silgen = NULL;
 
 	/* Stop if we're a zombie or need a soft hangup */
-	if (ast_test_flag(ast_channel_flags(c), AST_FLAG_ZOMBIE) || ast_check_hangup(c))
+	if (ast_channel_test_flag(c, AST_FLAG_ZOMBIE) || ast_check_hangup(c))
 		return -1;
 	if (!len)
 		return -1;
@@ -6645,7 +6648,7 @@
 	ao2_unlink(channels, original);
 	ao2_unlink(channels, clonechan);
 
-	moh_is_playing = ast_test_flag(ast_channel_flags(original), AST_FLAG_MOH);
+	moh_is_playing = ast_channel_test_flag(original, AST_FLAG_MOH);
 	if (moh_is_playing) {
 		/* Stop MOH on the old original channel. */
 		ast_moh_stop(original);
@@ -7122,7 +7125,7 @@
 	/* We have to pass AST_DEVICE_UNKNOWN here because it is entirely possible that the channel driver
 	 * for this channel is using the callback method for device state. If we pass in an actual state here
 	 * we override what they are saying the state is and things go amuck. */
-	ast_devstate_changed_literal(AST_DEVICE_UNKNOWN, (ast_test_flag(ast_channel_flags(chan), AST_FLAG_DISABLE_DEVSTATE_CACHE) ? AST_DEVSTATE_NOT_CACHABLE : AST_DEVSTATE_CACHABLE), name);
+	ast_devstate_changed_literal(AST_DEVICE_UNKNOWN, (ast_channel_test_flag(chan, AST_FLAG_DISABLE_DEVSTATE_CACHE) ? AST_DEVSTATE_NOT_CACHABLE : AST_DEVSTATE_CACHABLE), name);
 
 	return 0;
 }
@@ -10947,3 +10950,28 @@
 
 	return ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_CHANGED, topology, sizeof(topology));
 }
+
+void ast_channel_set_flag(struct ast_channel *chan, unsigned int flag)
+{
+	ast_channel_lock(chan);
+	ast_set_flag(ast_channel_flags(chan), flag);
+	ast_channel_unlock(chan);
+}
+
+unsigned int ast_channel_test_flag(struct ast_channel *chan, unsigned int flag)
+{
+	unsigned int set;
+
+	ast_channel_lock(chan);
+	set = ast_test_flag(ast_channel_flags(chan), flag);
+	ast_channel_unlock(chan);
+
+	return set;
+}
+
+void ast_channel_clear_flag(struct ast_channel *chan, unsigned int flag)
+{
+	ast_channel_lock(chan);
+	ast_clear_flag(ast_channel_flags(chan), flag);
+	ast_channel_unlock(chan);
+}
diff --git a/main/file.c b/main/file.c
index fb4ede6..48fb891 100644
--- a/main/file.c
+++ b/main/file.c
@@ -1269,7 +1269,7 @@
 		ast_debug(1, "Ooh, found a video stream, too, format %s\n", ast_format_get_name(vfs->fmt->format));
 	}
 
-	if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_MASQ_NOSTREAM))
+	if (ast_channel_test_flag(chan, AST_FLAG_MASQ_NOSTREAM))
 		fs->orig_chan_name = ast_strdup(ast_channel_name(chan));
 	if (ast_applystream(chan, fs))
 		return -1;
@@ -1543,9 +1543,9 @@
 		reverse = "";
 
 	/* Switch the channel to end DTMF frame only. waitstream_core doesn't care about the start of DTMF. */
-	ast_set_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_set_flag(c, AST_FLAG_END_DTMF_ONLY);
 
-	if (ast_test_flag(ast_channel_flags(c), AST_FLAG_MASQ_NOSTREAM))
+	if (ast_channel_test_flag(c, AST_FLAG_MASQ_NOSTREAM))
 		orig_chan_name = ast_strdupa(ast_channel_name(c));
 
 	if (ast_channel_stream(c) && cb) {
@@ -1575,7 +1575,7 @@
 			res = ast_waitfor(c, ms);
 			if (res < 0) {
 				ast_log(LOG_WARNING, "Select failed (%s)\n", strerror(errno));
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return res;
 			}
 		} else {
@@ -1586,11 +1586,11 @@
 				if (errno == EINTR)
 					continue;
 				ast_log(LOG_WARNING, "Wait failed (%s)\n", strerror(errno));
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return -1;
 			} else if (outfd > -1) { /* this requires cmdfd set */
 				/* The FD we were watching has something waiting */
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return 1;
 			}
 			/* if rchan is set, it is 'c' */
@@ -1599,7 +1599,7 @@
 		if (res > 0) {
 			struct ast_frame *fr = ast_read(c);
 			if (!fr) {
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return -1;
 			}
 			switch (fr->frametype) {
@@ -1610,7 +1610,7 @@
 						S_COR(ast_channel_caller(c)->id.number.valid, ast_channel_caller(c)->id.number.str, NULL))) {
 						res = fr->subclass.integer;
 						ast_frfree(fr);
-						ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+						ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 						return res;
 					}
 				} else {
@@ -1626,7 +1626,7 @@
 							"Break");
 
 						ast_frfree(fr);
-						ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+						ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 						return res;
 					}
 				}
@@ -1643,7 +1643,7 @@
 						"Break");
 					res = fr->subclass.integer;
 					ast_frfree(fr);
-					ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+					ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 					return res;
 				case AST_CONTROL_STREAM_REVERSE:
 					if (!skip_ms) {
@@ -1661,7 +1661,7 @@
 				case AST_CONTROL_BUSY:
 				case AST_CONTROL_CONGESTION:
 					ast_frfree(fr);
-					ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+					ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 					return -1;
 				case AST_CONTROL_RINGING:
 				case AST_CONTROL_ANSWER:
@@ -1698,7 +1698,7 @@
 		ast_sched_runq(ast_channel_sched(c));
 	}
 
-	ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 
 	return (err || ast_channel_softhangup_internal_flag(c)) ? -1 : 0;
 }
diff --git a/main/manager.c b/main/manager.c
index c592fbd..dfb0d96 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -4847,14 +4847,10 @@
 
 	/* Release the bridge wait. */
 	if (chan1_wait) {
-		ast_channel_lock(chan);
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
-		ast_channel_unlock(chan);
+		ast_channel_clear_flag(chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
 	}
 	if (chan2_wait) {
-		ast_channel_lock(chan2);
-		ast_clear_flag(ast_channel_flags(chan2), AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
-		ast_channel_unlock(chan2);
+		ast_channel_clear_flag(chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
 	}
 
 	chan2 = ast_channel_unref(chan2);
diff --git a/main/pbx.c b/main/pbx.c
index 28027c0..6580822 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -4300,8 +4300,10 @@
 	ast_channel_pbx(c)->rtimeoutms = 10000;
 	ast_channel_pbx(c)->dtimeoutms = 5000;
 
+	ast_channel_lock(c);
 	autoloopflag = ast_test_flag(ast_channel_flags(c), AST_FLAG_IN_AUTOLOOP);	/* save value to restore at the end */
 	ast_set_flag(ast_channel_flags(c), AST_FLAG_IN_AUTOLOOP);
+	ast_channel_unlock(c);
 
 	if (ast_strlen_zero(ast_channel_exten(c))) {
 		/* If not successful fall back to 's' - but only if there is no given exten  */
@@ -4537,7 +4539,7 @@
 
 	if (!args || !args->no_hangup_chan) {
 		ast_softhangup(c, AST_SOFTHANGUP_APPUNLOAD);
-		if (!ast_test_flag(ast_channel_flags(c), AST_FLAG_BRIDGE_HANGUP_RUN)
+		if (!ast_channel_test_flag(c, AST_FLAG_BRIDGE_HANGUP_RUN)
 			&& ast_exists_extension(c, ast_channel_context(c), "h", 1,
 				S_COR(ast_channel_caller(c)->id.number.valid,
 					ast_channel_caller(c)->id.number.str, NULL))) {
@@ -4546,8 +4548,10 @@
 		ast_pbx_hangup_handler_run(c);
 	}
 
+	ast_channel_lock(c);
 	ast_set2_flag(ast_channel_flags(c), autoloopflag, AST_FLAG_IN_AUTOLOOP);
 	ast_clear_flag(ast_channel_flags(c), AST_FLAG_BRIDGE_HANGUP_RUN); /* from one round to the next, make sure this gets cleared */
+	ast_channel_unlock(c);
 	pbx_destroy(ast_channel_pbx(c));
 	ast_channel_pbx_set(c, NULL);
 
diff --git a/main/pbx_builtins.c b/main/pbx_builtins.c
index 20fdb4c..117fba5 100644
--- a/main/pbx_builtins.c
+++ b/main/pbx_builtins.c
@@ -1129,7 +1129,7 @@
 	 * users can EXEC Background and reasonably expect that the DTMF code will
 	 * be returned (see #16434).
 	 */
-	if (!ast_test_flag(ast_channel_flags(chan), AST_FLAG_DISABLE_WORKAROUNDS)
+	if (!ast_channel_test_flag(chan, AST_FLAG_DISABLE_WORKAROUNDS)
 		&& (exten[0] = res)
 		&& ast_canmatch_extension(chan, args.context, exten, 1,
 			S_COR(ast_channel_caller(chan)->id.number.valid, ast_channel_caller(chan)->id.number.str, NULL))
diff --git a/res/res_agi.c b/res/res_agi.c
index 557f349..dd34114 100644
--- a/res/res_agi.c
+++ b/res/res_agi.c
@@ -3119,12 +3119,14 @@
 	ast_verb(3, "AGI Script Executing Application: (%s) Options: (%s)\n", argv[1], argc >= 3 ? argv[2] : "");
 
 	if ((app_to_exec = pbx_findapp(argv[1]))) {
+		ast_channel_lock(chan);
 		if (!(workaround = ast_test_flag(ast_channel_flags(chan), AST_FLAG_DISABLE_WORKAROUNDS))) {
 			ast_set_flag(ast_channel_flags(chan), AST_FLAG_DISABLE_WORKAROUNDS);
 		}
+		ast_channel_unlock(chan);
 		res = pbx_exec(chan, app_to_exec, argc == 2 ? "" : argv[2]);
 		if (!workaround) {
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_DISABLE_WORKAROUNDS);
+			ast_channel_clear_flag(chan, AST_FLAG_DISABLE_WORKAROUNDS);
 		}
 	} else {
 		ast_log(LOG_WARNING, "Could not find application (%s)\n", argv[1]);
diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index 71f4691..be50e9c 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -1558,8 +1558,10 @@
 		}
 	}
 	if (!res) {
+		ast_channel_lock(chan);
 		ast_channel_latest_musicclass_set(chan, mohclass->name);
 		ast_set_flag(ast_channel_flags(chan), AST_FLAG_MOH);
+		ast_channel_unlock(chan);
 	}
 
 	mohclass = mohclass_unref(mohclass, "unreffing local reference to mohclass in local_ast_moh_start");
@@ -1569,10 +1571,10 @@
 
 static void local_ast_moh_stop(struct ast_channel *chan)
 {
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_MOH);
 	ast_deactivate_generator(chan);
 
 	ast_channel_lock(chan);
+	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_MOH);
 	if (ast_channel_music_state(chan)) {
 		if (ast_channel_stream(chan)) {
 			ast_closestream(ast_channel_stream(chan));

-- 
To view, visit https://gerrit.asterisk.org/5625
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I489280662dba0f4c50981bfc5b5a7073fef2db10
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>



More information about the asterisk-code-review mailing list