ao2_iterator: Mini-audit of the ao2_iterator loops in the new code files.
authorRichard Mudgett <rmudgett@digium.com>
Fri, 20 Dec 2013 20:00:50 +0000 (20:00 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Fri, 20 Dec 2013 20:00:50 +0000 (20:00 +0000)
* 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

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

res/ari/resource_bridges.c
res/ari/resource_channels.c
res/ari/resource_endpoints.c
res/parking/parking_manager.c
res/res_pjsip/location.c
tests/test_cel.c
tests/test_scoped_lock.c
tests/test_stasis.c

index ed3eba8..16f2627 100644 (file)
@@ -682,6 +682,7 @@ void ast_ari_bridges_list(struct ast_variable *headers,
                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;
                }
index 08ef154..667ea73 100644 (file)
@@ -663,8 +663,8 @@ void ast_ari_channels_list(struct ast_variable *headers,
                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 @@ void ast_ari_channels_list(struct ast_variable *headers,
                        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;
                }
index c37f496..7dab25b 100644 (file)
@@ -74,12 +74,14 @@ void ast_ari_endpoints_list(struct ast_variable *headers,
                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 @@ void ast_ari_endpoints_list_by_tech(struct ast_variable *headers,
                r = ast_json_array_append(
                        json, json_endpoint);
                if (r != 0) {
+                       ao2_iterator_destroy(&i);
                        ast_ari_response_alloc_failed(response);
                        return;
                }
index 9d8c236..70a230c 100644 (file)
@@ -229,7 +229,7 @@ static struct ast_str *manager_build_parked_call_string(const struct ast_parked_
        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 @@ static int manager_parking_status_single_lot(struct mansession *s, const struct
        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 @@ static int manager_parking_status_single_lot(struct mansession *s, const struct
 
                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 @@ static int manager_parking_status_single_lot(struct mansession *s, const struct
 
                ao2_ref(curuser, -1);
        }
-
        ao2_iterator_destroy(&iter_users);
 
        astman_append(s,
@@ -282,11 +284,9 @@ static int manager_parking_status_single_lot(struct mansession *s, const struct
                "%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 @@ static int manager_parking_status_all_lots(struct mansession *s, const struct me
        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;
        }
 
-       iter_lots = ao2_iterator_init(lot_container, 0);
-
        astman_send_ack(s, m, "Parked calls will follow");
 
+       iter_lots = ao2_iterator_init(lot_container, 0);
        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 @@ static int manager_parking_status_all_lots(struct mansession *s, const struct me
 
                        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 @@ static int manager_parking_status_all_lots(struct mansession *s, const struct me
                ao2_iterator_destroy(&iter_users);
                ao2_ref(curlot, -1);
        }
-
        ao2_iterator_destroy(&iter_lots);
 
        astman_append(s,
@@ -346,8 +351,6 @@ static int manager_parking_status_all_lots(struct mansession *s, const struct me
                "%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 @@ static int manager_parking_status(struct mansession *s, const struct message *m)
        }
 
        if (!ast_strlen_zero(lot_name)) {
-               return manager_parking_status_single_lot(s, m, id_text, lot_name);
+               manager_parking_status_single_lot(s, m, id_text, lot_name);
+       } else {
+               manager_parking_status_all_lots(s, m, id_text);
        }
 
-       return 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 @@ static int manager_parking_lot_list(struct mansession *s, const struct message *
        }
 
        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 @@ static int manager_parking_lot_list(struct mansession *s, const struct message *
                "%s"
                "\r\n",id_text);
 
-       return RESULT_SUCCESS;
+       return 0;
 }
 
 static int manager_park(struct mansession *s, const struct message *m)
index ddd8461..52f69cf 100644 (file)
@@ -304,6 +304,7 @@ int ast_sip_for_each_contact(const struct ast_sip_aor *aor,
 
                ao2_ref(contact, -1);
                if (res) {
+                       ao2_iterator_destroy(&i);
                        return -1;
                }
        }
index 62a3288..f509cc5 100644 (file)
@@ -1906,8 +1906,10 @@ static int dump_event(struct ast_test *test, struct ast_event *event)
 
 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 @@ static int check_events(struct ast_test *test, struct ao2_container *local_expec
                debug = 1;
        }
 
-       expected_it = ao2_iterator_init(local_expected, 0);
        received_it = ao2_iterator_init(local_received, 0);
+       expected_it = ao2_iterator_init(local_expected, 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 @@ static int check_events(struct ast_test *test, struct ao2_container *local_expec
                        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 @@ static int check_events(struct ast_test *test, struct ao2_container *local_expec
                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;
index 2fcaae2..1881bce 100644 (file)
@@ -254,6 +254,7 @@ AST_TEST_DEFINE(cleanup_order)
                        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");
index 9ee1684..7a297ce 100644 (file)
@@ -818,6 +818,7 @@ AST_TEST_DEFINE(cache_dump)
                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 @@ AST_TEST_DEFINE(cache_dump)
                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 @@ AST_TEST_DEFINE(cache_dump)
                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);