[asterisk-commits] rmudgett: trunk r404436 - in /trunk: ./ res/ari/ res/parking/ res/res_pjsip/ ...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Dec 20 14:00:53 CST 2013


Author: rmudgett
Date: Fri Dec 20 14:00:50 2013
New Revision: 404436

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=404436
Log:
ao2_iterator: Mini-audit of the ao2_iterator loops in the new code files.

* Fixed several places where ao2_iterator_destroy() was not called.

* Fixed several iterator loop object variable reference problems.

* Fixed res_parking AMI actions returning non-zero.  Only the AMI logoff
action can return non-zero.

Review: https://reviewboard.asterisk.org/r/3087/
........

Merged revisions 404434 from http://svn.asterisk.org/svn/asterisk/branches/12

Modified:
    trunk/   (props changed)
    trunk/res/ari/resource_bridges.c
    trunk/res/ari/resource_channels.c
    trunk/res/ari/resource_endpoints.c
    trunk/res/parking/parking_manager.c
    trunk/res/res_pjsip/location.c
    trunk/tests/test_cel.c
    trunk/tests/test_scoped_lock.c
    trunk/tests/test_stasis.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.

Modified: trunk/res/ari/resource_bridges.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/ari/resource_bridges.c?view=diff&rev=404436&r1=404435&r2=404436
==============================================================================
--- trunk/res/ari/resource_bridges.c (original)
+++ trunk/res/ari/resource_bridges.c Fri Dec 20 14:00:50 2013
@@ -682,6 +682,7 @@
 		struct ast_json *json_bridge = ast_bridge_snapshot_to_json(snapshot, stasis_app_get_sanitizer());
 
 		if (!json_bridge || ast_json_array_append(json, json_bridge)) {
+			ao2_iterator_destroy(&i);
 			ast_ari_response_alloc_failed(response);
 			return;
 		}

Modified: trunk/res/ari/resource_channels.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/ari/resource_channels.c?view=diff&rev=404436&r1=404435&r2=404436
==============================================================================
--- trunk/res/ari/resource_channels.c (original)
+++ trunk/res/ari/resource_channels.c Fri Dec 20 14:00:50 2013
@@ -663,8 +663,8 @@
 		return;
 	}
 
-	for (i = ao2_iterator_init(snapshots, 0);
-		(obj = ao2_iterator_next(&i)); ao2_cleanup(obj)) {
+	i = ao2_iterator_init(snapshots, 0);
+	while ((obj = ao2_iterator_next(&i))) {
 		RAII_VAR(struct stasis_message *, msg, obj, ao2_cleanup);
 		struct ast_channel_snapshot *snapshot = stasis_message_data(msg);
 		int r;
@@ -678,7 +678,6 @@
 			json, ast_channel_snapshot_to_json(snapshot, NULL));
 		if (r != 0) {
 			ast_ari_response_alloc_failed(response);
-			ao2_cleanup(obj);
 			ao2_iterator_destroy(&i);
 			return;
 		}

Modified: trunk/res/ari/resource_endpoints.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/ari/resource_endpoints.c?view=diff&rev=404436&r1=404435&r2=404436
==============================================================================
--- trunk/res/ari/resource_endpoints.c (original)
+++ trunk/res/ari/resource_endpoints.c Fri Dec 20 14:00:50 2013
@@ -74,12 +74,14 @@
 		int r;
 
 		if (!json_endpoint) {
+			ao2_iterator_destroy(&i);
 			return;
 		}
 
 		r = ast_json_array_append(
 			json, json_endpoint);
 		if (r != 0) {
+			ao2_iterator_destroy(&i);
 			ast_ari_response_alloc_failed(response);
 			return;
 		}
@@ -144,6 +146,7 @@
 		r = ast_json_array_append(
 			json, json_endpoint);
 		if (r != 0) {
+			ao2_iterator_destroy(&i);
 			ast_ari_response_alloc_failed(response);
 			return;
 		}

Modified: trunk/res/parking/parking_manager.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/parking/parking_manager.c?view=diff&rev=404436&r1=404435&r2=404436
==============================================================================
--- trunk/res/parking/parking_manager.c (original)
+++ trunk/res/parking/parking_manager.c Fri Dec 20 14:00:50 2013
@@ -229,7 +229,7 @@
 	return out;
 }
 
-static int manager_parking_status_single_lot(struct mansession *s, const struct message *m, const char *id_text, const char *lot_name)
+static void manager_parking_status_single_lot(struct mansession *s, const struct message *m, const char *id_text, const char *lot_name)
 {
 	RAII_VAR(struct parking_lot *, curlot, NULL, ao2_cleanup);
 	struct parked_user *curuser;
@@ -237,10 +237,9 @@
 	int total = 0;
 
 	curlot = parking_lot_find_by_name(lot_name);
-
 	if (!curlot) {
 		astman_send_error(s, m, "Requested parking lot could not be found.");
-		return RESULT_SUCCESS;
+		return;
 	}
 
 	astman_send_ack(s, m, "Parked calls will follow");
@@ -252,14 +251,18 @@
 
 		payload = parked_call_payload_from_parked_user(curuser, PARKED_CALL);
 		if (!payload) {
+			ao2_ref(curuser, -1);
+			ao2_iterator_destroy(&iter_users);
 			astman_send_error(s, m, "Failed to retrieve parking data about a parked user.");
-			return RESULT_FAILURE;
+			return;
 		}
 
 		parked_call_string = manager_build_parked_call_string(payload);
 		if (!parked_call_string) {
-			astman_send_error(s, m, "Failed to retrieve parkingd ata about a parked user.");
-			return RESULT_FAILURE;
+			ao2_ref(curuser, -1);
+			ao2_iterator_destroy(&iter_users);
+			astman_send_error(s, m, "Failed to retrieve parking data about a parked user.");
+			return;
 		}
 
 		total++;
@@ -273,7 +276,6 @@
 
 		ao2_ref(curuser, -1);
 	}
-
 	ao2_iterator_destroy(&iter_users);
 
 	astman_append(s,
@@ -282,11 +284,9 @@
 		"%s"
 		"\r\n",
 		total, id_text);
-
-	return RESULT_SUCCESS;
-}
-
-static int manager_parking_status_all_lots(struct mansession *s, const struct message *m, const char *id_text)
+}
+
+static void manager_parking_status_all_lots(struct mansession *s, const struct message *m, const char *id_text)
 {
 	struct parked_user *curuser;
 	struct ao2_container *lot_container;
@@ -296,17 +296,15 @@
 	int total = 0;
 
 	lot_container = get_parking_lot_container();
-
 	if (!lot_container) {
 		ast_log(LOG_ERROR, "Failed to obtain parking lot list. Action canceled.\n");
 		astman_send_error(s, m, "Could not create parking lot list");
-		return RESULT_SUCCESS;
-	}
+		return;
+	}
+
+	astman_send_ack(s, m, "Parked calls will follow");
 
 	iter_lots = ao2_iterator_init(lot_container, 0);
-
-	astman_send_ack(s, m, "Parked calls will follow");
-
 	while ((curlot = ao2_iterator_next(&iter_lots))) {
 		iter_users = ao2_iterator_init(curlot->parked_users, 0);
 		while ((curuser = ao2_iterator_next(&iter_users))) {
@@ -315,12 +313,20 @@
 
 			payload = parked_call_payload_from_parked_user(curuser, PARKED_CALL);
 			if (!payload) {
-				return RESULT_FAILURE;
+				ao2_ref(curuser, -1);
+				ao2_iterator_destroy(&iter_users);
+				ao2_ref(curlot, -1);
+				ao2_iterator_destroy(&iter_lots);
+				return;
 			}
 
 			parked_call_string = manager_build_parked_call_string(payload);
-			if (!payload) {
-				return RESULT_FAILURE;
+			if (!parked_call_string) {
+				ao2_ref(curuser, -1);
+				ao2_iterator_destroy(&iter_users);
+				ao2_ref(curlot, -1);
+				ao2_iterator_destroy(&iter_lots);
+				return;
 			}
 
 			total++;
@@ -337,7 +343,6 @@
 		ao2_iterator_destroy(&iter_users);
 		ao2_ref(curlot, -1);
 	}
-
 	ao2_iterator_destroy(&iter_lots);
 
 	astman_append(s,
@@ -346,8 +351,6 @@
 		"%s"
 		"\r\n",
 		total, id_text);
-
-	return RESULT_SUCCESS;
 }
 
 static int manager_parking_status(struct mansession *s, const struct message *m)
@@ -361,11 +364,12 @@
 	}
 
 	if (!ast_strlen_zero(lot_name)) {
-		return manager_parking_status_single_lot(s, m, id_text, lot_name);
-	}
-
-	return manager_parking_status_all_lots(s, m, id_text);
-
+		manager_parking_status_single_lot(s, m, id_text, lot_name);
+	} else {
+		manager_parking_status_all_lots(s, m, id_text);
+	}
+
+	return 0;
 }
 
 static int manager_append_event_parking_lot_data_cb(void *obj, void *arg, void *data, int flags)
@@ -401,11 +405,10 @@
 	}
 
 	lot_container = get_parking_lot_container();
-
 	if (!lot_container) {
 		ast_log(LOG_ERROR, "Failed to obtain parking lot list. Action canceled.\n");
 		astman_send_error(s, m, "Could not create parking lot list");
-		return -1;
+		return 0;
 	}
 
 	astman_send_ack(s, m, "Parking lots will follow");
@@ -417,7 +420,7 @@
 		"%s"
 		"\r\n",id_text);
 
-	return RESULT_SUCCESS;
+	return 0;
 }
 
 static int manager_park(struct mansession *s, const struct message *m)

Modified: trunk/res/res_pjsip/location.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_pjsip/location.c?view=diff&rev=404436&r1=404435&r2=404436
==============================================================================
--- trunk/res/res_pjsip/location.c (original)
+++ trunk/res/res_pjsip/location.c Fri Dec 20 14:00:50 2013
@@ -304,6 +304,7 @@
 
 		ao2_ref(contact, -1);
 		if (res) {
+			ao2_iterator_destroy(&i);
 			return -1;
 		}
 	}

Modified: trunk/tests/test_cel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_cel.c?view=diff&rev=404436&r1=404435&r2=404436
==============================================================================
--- trunk/tests/test_cel.c (original)
+++ trunk/tests/test_cel.c Fri Dec 20 14:00:50 2013
@@ -1906,8 +1906,10 @@
 
 static int check_events(struct ast_test *test, struct ao2_container *local_expected, struct ao2_container *local_received)
 {
-	struct ao2_iterator expected_it, received_it;
-	struct ast_event *rx_event, *ex_event;
+	struct ao2_iterator received_it;
+	struct ao2_iterator expected_it;
+	RAII_VAR(struct ast_event *, rx_event, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_event *, ex_event, NULL, ao2_cleanup);
 	int debug = 0;
 
 	if (ao2_container_count(local_expected) != ao2_container_count(local_received)) {
@@ -1918,12 +1920,14 @@
 		debug = 1;
 	}
 
+	received_it = ao2_iterator_init(local_received, 0);
 	expected_it = ao2_iterator_init(local_expected, 0);
-	received_it = ao2_iterator_init(local_received, 0);
 	rx_event = ao2_iterator_next(&received_it);
 	ex_event = ao2_iterator_next(&expected_it);
 	while (rx_event && ex_event) {
 		if (!events_are_equal(test, rx_event, ex_event)) {
+			ao2_iterator_destroy(&received_it);
+			ao2_iterator_destroy(&expected_it);
 			ast_test_status_update(test, "Received event:\n");
 			dump_event(test, rx_event);
 			ast_test_status_update(test, "Expected event:\n");
@@ -1931,7 +1935,9 @@
 			return -1;
 		}
 		if (debug) {
-			ast_test_status_update(test, "Compared events successfully%s\n", ast_event_get_type(ex_event) == AST_EVENT_CUSTOM ? " (wildcard match)" : "");
+			ast_test_status_update(test, "Compared events successfully%s\n",
+				ast_event_get_type(ex_event) == AST_EVENT_CUSTOM
+					? " (wildcard match)" : "");
 			dump_event(test, rx_event);
 		}
 		ao2_cleanup(rx_event);
@@ -1939,17 +1945,17 @@
 		rx_event = ao2_iterator_next(&received_it);
 		ex_event = ao2_iterator_next(&expected_it);
 	}
+	ao2_iterator_destroy(&received_it);
+	ao2_iterator_destroy(&expected_it);
 
 	if (rx_event) {
 		ast_test_status_update(test, "Received event:\n");
 		dump_event(test, rx_event);
-		ao2_cleanup(rx_event);
 		return -1;
 	}
 	if (ex_event) {
 		ast_test_status_update(test, "Expected event:\n");
 		dump_event(test, ex_event);
-		ao2_cleanup(ex_event);
 		return -1;
 	}
 	return 0;

Modified: trunk/tests/test_scoped_lock.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_scoped_lock.c?view=diff&rev=404436&r1=404435&r2=404436
==============================================================================
--- trunk/tests/test_scoped_lock.c (original)
+++ trunk/tests/test_scoped_lock.c Fri Dec 20 14:00:50 2013
@@ -254,6 +254,7 @@
 			res = AST_TEST_FAIL;
 		}
 	}
+	ao2_iterator_destroy(&iter);
 
 	if (object->reffed || object->locked) {
 		ast_log(LOG_ERROR, "Test failed due to out of order cleanups\n");

Modified: trunk/tests/test_stasis.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_stasis.c?view=diff&rev=404436&r1=404435&r2=404436
==============================================================================
--- trunk/tests/test_stasis.c (original)
+++ trunk/tests/test_stasis.c Fri Dec 20 14:00:50 2013
@@ -818,6 +818,7 @@
 		RAII_VAR(struct stasis_message *, actual_cache_entry, obj, ao2_cleanup);
 		ast_test_validate(test, actual_cache_entry == test_message1_1 || actual_cache_entry == test_message2_1);
 	}
+	ao2_iterator_destroy(&i);
 
 	/* Update snapshot 2 */
 	test_message2_2 = cache_test_message_create(cache_type, "2", "2");
@@ -836,6 +837,7 @@
 		RAII_VAR(struct stasis_message *, actual_cache_entry, obj, ao2_cleanup);
 		ast_test_validate(test, actual_cache_entry == test_message1_1 || actual_cache_entry == test_message2_2);
 	}
+	ao2_iterator_destroy(&i);
 
 	/* Clear snapshot 1 */
 	test_message1_clear = stasis_cache_clear_create(test_message1_1);
@@ -854,6 +856,7 @@
 		RAII_VAR(struct stasis_message *, actual_cache_entry, obj, ao2_cleanup);
 		ast_test_validate(test, actual_cache_entry == test_message2_2);
 	}
+	ao2_iterator_destroy(&i);
 
 	/* Dump the cache to ensure that it has no subscription change items in it since those aren't cached */
 	ao2_cleanup(cache_dump);




More information about the asterisk-commits mailing list