[asterisk-commits] mmichelson: branch group/CCSS r225579 - /team/group/CCSS/main/ccss.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Oct 22 19:08:11 CDT 2009


Author: mmichelson
Date: Thu Oct 22 19:08:07 2009
New Revision: 225579

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=225579
Log:
Handle state change validation in a single function.

Yay for cleaner code! The problem is that somewhere along
the way I have accidentally put in a new memory leak.

Got to get that figured out tomorrow morning.


Modified:
    team/group/CCSS/main/ccss.c

Modified: team/group/CCSS/main/ccss.c
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/main/ccss.c?view=diff&rev=225579&r1=225578&r2=225579
==============================================================================
--- team/group/CCSS/main/ccss.c (original)
+++ team/group/CCSS/main/ccss.c Thu Oct 22 19:08:07 2009
@@ -2319,17 +2319,84 @@
 	char debug[1];
 };
 
+static int is_state_change_valid(enum cc_state *current_state, const enum cc_state new_state, struct ast_cc_agent *agent)
+{
+	int is_valid = 0;
+	switch (new_state) {
+	case CC_AVAILABLE:
+		ast_log_dynamic_level(cc_logger_level, "Asked to change to state %d? That should never happen.\n", new_state);
+		break;
+	case CC_CALLER_OFFERED:
+		if (*current_state == CC_AVAILABLE) {
+			is_valid = 1;
+		}
+		break;
+	case CC_CALLER_REQUESTED:
+		if (*current_state == CC_CALLER_OFFERED ||
+				(*current_state == CC_AVAILABLE && ast_test_flag(agent, AST_CC_AGENT_SKIP_OFFER))) {
+			is_valid = 1;
+		}
+		break;
+	case CC_ACTIVE:
+		if (*current_state == CC_CALLER_REQUESTED || *current_state == CC_CALLER_BUSY) {
+			is_valid = 1;
+		}
+		break;
+	case CC_CALLEE_READY:
+		if (*current_state == CC_ACTIVE) {
+			is_valid = 1;
+		}
+		break;
+	case CC_CALLER_BUSY:
+		if (*current_state == CC_CALLEE_READY) {
+			is_valid = 1;
+		}
+		break;
+	case CC_RECALLING:
+		if (*current_state == CC_CALLEE_READY) {
+			is_valid = 1;
+		}
+		break;
+	case CC_COMPLETE:
+		if (*current_state == CC_RECALLING) {
+			is_valid = 1;
+		}
+		break;
+	case CC_FAILED:
+		is_valid = 1;
+		break;
+	default:
+		ast_log_dynamic_level(cc_logger_level, "Asked to change to unknown state %d\n", new_state);
+	}
+
+	if (is_valid) {
+		*current_state = new_state;
+	}
+
+	return is_valid;
+}
+
 static int cc_do_state_change(void *datap)
 {
 	struct cc_state_change_args *args = datap;
 	struct cc_core_instance *core_instance;
 	struct cc_core_instance finder = { .core_id = args->core_id };
+	enum cc_state previous_state;
 
 	ast_log_dynamic_level(cc_logger_level, "State change to %d requested. Reason: %s\n", args->state, args->debug);
 
 	if (!(core_instance = ao2_t_find(cc_core_instances, &finder, OBJ_POINTER, "Find core instance so state change can occur"))) {
 		ast_log_dynamic_level(cc_logger_level, "Unable to find core instance %d\n", args->core_id);
 		ast_free(args);
+		return -1;
+	}
+
+	previous_state = core_instance->current_state;
+	if (!is_state_change_valid(&core_instance->current_state, args->state, core_instance->agent)) {
+		ast_log_dynamic_level(cc_logger_level, "Invalid state change requested. Cannot go from %s to %s\n",
+				cc_state_to_string(core_instance->current_state), cc_state_to_string(args->state));
+		ast_free(args);
+		cc_unref(core_instance, "Unref core instance from when it was found earlier");
 		return -1;
 	}
 
@@ -2339,11 +2406,6 @@
 		ast_log(LOG_WARNING, "Someone requested to change to CC_AVAILABLE? Ignoring.\n");
 		break;
 	case CC_CALLER_OFFERED:
-		if (core_instance->current_state != CC_AVAILABLE) {
-			ast_log_dynamic_level(cc_logger_level, "Invalid state change request. Cannot go from %d to %d\n",
-					core_instance->current_state, args->state);
-			break;
-		}
 		if (core_instance->agent->callbacks->start_offer_timer(core_instance->agent)) {
 			ast_cc_failed(core_instance->core_id, "Failed to start the offer timer\n");
 			break;
@@ -2354,25 +2416,13 @@
 			"Expires: %u\r\n",
 			core_instance->core_id, core_instance->agent->interface, core_instance->agent->cc_params->cc_offer_timer);
 		ast_log_dynamic_level(cc_logger_level, "Started the offer timer for the agent!\n");
-		core_instance->current_state = args->state;
 		break;
 	case CC_CALLER_REQUESTED:
-		/* An English translation of this is that the only valid state change here is either if the current
-		 * state is CC_CALLER_OFFERED or if the current state is CC_AVAILABLE and the type of the agent
-		 * in use is generic.
-		 */
-		if (!(core_instance->current_state == CC_CALLER_OFFERED ||
-				(core_instance->current_state == CC_AVAILABLE && ast_test_flag(core_instance->agent, AST_CC_AGENT_SKIP_OFFER)))) {
-			ast_log_dynamic_level(cc_logger_level, "Invalid state change request. Cannot go from %d to %d\n",
-					core_instance->current_state, args->state);
-			break;
-		}
 		if (!ast_cc_request_within_limits()) {
 			ast_log(LOG_WARNING, "Cannot request CC since there is no more room for requests\n");
 			ast_cc_failed(core_instance->core_id, "Too many requests in the system");
 			break;
 		}
-		core_instance->current_state = args->state;
 		core_instance->agent->callbacks->stop_offer_timer(core_instance->agent);
 		cc_monitor_tree_init(args->core_id);
 		/* It doesn't matter what service we state for the root monitor, so we just use AST_CC_NONE */
@@ -2388,33 +2438,23 @@
 		 * 2. Caller became available, call agent's stop_monitoring callback and
 		 *    call monitor's unsuspend callback.
 		 */
-		if (core_instance->current_state == CC_CALLER_REQUESTED) {
-			core_instance->current_state = args->state;
+		if (previous_state == CC_CALLER_REQUESTED) {
 			core_instance->agent->callbacks->ack(core_instance->agent);
 			manager_event(EVENT_FLAG_CC, "CCRequestAcknowledged",
 				"CoreID: %d\r\n", core_instance->core_id);
-		} else if (core_instance->current_state == CC_CALLER_BUSY) {
-			core_instance->current_state = args->state;
+		} else if (previous_state == CC_CALLER_BUSY) {
 			core_instance->agent->callbacks->stop_monitoring(core_instance->agent);
 			manager_event(EVENT_FLAG_CC, "CCCallerStopMonitoring",
 				"CoreID: %d\r\n"
 				"Caller: %s\r\n",
 				core_instance->core_id, core_instance->agent->interface);
 			core_instance->monitor->callbacks->unsuspend(core_instance->monitor, core_instance->core_id);
-		} else {
-			ast_log_dynamic_level(cc_logger_level, "Invalid state change request. Cannot go from %d to %d\n",
-					core_instance->current_state, args->state);
-		}
+		}
+		/* Not possible for previous_state to be anything else due to the is_state_change_valid check at the beginning */
 		break;
 	case CC_CALLEE_READY:
 		/* Callee has become available, call agent's status request callback
 		 */
-		if (core_instance->current_state != CC_ACTIVE) {
-			ast_log_dynamic_level(cc_logger_level, "Invalid state change request. Cannot go from %d to %d\n",
-					core_instance->current_state, args->state);
-			break;
-		}
-		core_instance->current_state = args->state;
 		if (core_instance->agent->callbacks->status_request(core_instance->agent) == AST_DEVICE_NOT_INUSE) {
 			ast_log_dynamic_level(cc_logger_level, "Both parties are ready. Attempting recall\n");
 			cc_request_state_change(CC_RECALLING, core_instance->core_id, "Both parties are available\n");
@@ -2427,12 +2467,6 @@
 		/* Callee was available, but caller was busy, call agent's begin_monitoring callback
 		 * and call monitor's suspend callback.
 		 */
-		if (core_instance->current_state != CC_CALLEE_READY) {
-			ast_log_dynamic_level(cc_logger_level, "Invalid state change request. Cannot go from %d to %d\n",
-					core_instance->current_state, args->state);
-			break;
-		}
-		core_instance->current_state = args->state;
 		core_instance->monitor->callbacks->suspend(core_instance->monitor, core_instance->core_id);
 		core_instance->agent->callbacks->start_monitoring(core_instance->agent);
 		manager_event(EVENT_FLAG_CC, "CCCallerStartMonitoring",
@@ -2443,12 +2477,6 @@
 	case CC_RECALLING:
 		/* Both caller and callee are available, call agent's recall callback
 		 */
-		if (core_instance->current_state != CC_CALLEE_READY) {
-			ast_log_dynamic_level(cc_logger_level, "Invalid state change request. Cannot go from %d to %d\n",
-					core_instance->current_state, args->state);
-			break;
-		}
-		core_instance->current_state = args->state;
 		core_instance->monitor->callbacks->cancel_available_timer(core_instance->monitor, core_instance->core_id, NULL);
 		core_instance->agent->callbacks->recall(core_instance->agent);
 		manager_event(EVENT_FLAG_CC, "CCCallerRecalling",
@@ -2459,12 +2487,6 @@
 	case CC_COMPLETE:
 		/* Recall has made progress, call agent and monitor destructor functions
 		 */
-		if (core_instance->current_state != CC_RECALLING) {
-			ast_log_dynamic_level(cc_logger_level, "Invalid state change request. Cannot go from %d to %d\n",
-					core_instance->current_state, args->state);
-			break;
-		}
-		core_instance->current_state = args->state;
 		manager_event(EVENT_FLAG_CC, "CCRecallComplete",
 			"CoreID: %d\r\n"
 			"Caller: %s\r\n",




More information about the asterisk-commits mailing list