Change ast_hangup() to return void and be NULL safe.

Since ast_hangup() is effectively a channel destructor, it should be a
void function.

* Make the few silly callers checking the return value no longer do so.
Only the CDR and CEL unit tests checked the return value.

* Make all callers take advantage of the NULL safe change and remove the
NULL check before the call.

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

diff --git a/addons/chan_mobile.c b/addons/chan_mobile.c
index 26e4abd..f5d5b4e 100644 (file)
--- a/addons/chan_mobile.c
+++ b/addons/chan_mobile.c
@@ -1339,9 +1339,7 @@ static int mbl_queue_hangup(struct mbl_pvt *pvt)
 
 static int mbl_ast_hangup(struct mbl_pvt *pvt)
 {
-       if (pvt->owner) {
-               ast_hangup(pvt->owner);
-       }
+       ast_hangup(pvt->owner);
        return 0;
 }
 

diff --git a/apps/app_meetme.c b/apps/app_meetme.c
index 7071960..0dee926 100644 (file)
--- a/apps/app_meetme.c
+++ b/apps/app_meetme.c
@@ -2368,10 +2368,8 @@ static int conf_free(struct ast_conference *conf)
 
        if (conf->origframe)
                ast_frfree(conf->origframe);
-       if (conf->lchan)
-               ast_hangup(conf->lchan);
-       if (conf->chan)
-               ast_hangup(conf->chan);
+       ast_hangup(conf->lchan);
+       ast_hangup(conf->chan);
        if (conf->fd >= 0)
                close(conf->fd);
        if (conf->recordingfilename) {

diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index 95265b5..df7f1b2 100644 (file)
--- a/apps/app_voicemail.c
+++ b/apps/app_voicemail.c
@@ -13780,9 +13780,7 @@ AST_TEST_DEFINE(test_voicemail_vmsayname)
 
 exit_vmsayname_test:
 
-       if (test_channel1) {
-               ast_hangup(test_channel1);
-       }
+       ast_hangup(test_channel1);
 
        return res ? AST_TEST_FAIL : AST_TEST_PASS;
 }

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 77629b7..94053ee 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -1427,9 +1427,9 @@ const struct ast_channel_tech *ast_get_channel_tech(const char *name);
  * performs all stream stopping, etc, on the channel that needs to end.
  * chan is no longer valid after this call.
  * \param chan channel to hang up
- * \return Returns 0 on success, -1 on failure.
+ * \return Nothing
  */
-int ast_hangup(struct ast_channel *chan);
+void ast_hangup(struct ast_channel *chan);
 
 /*!
  * \brief Softly hangup up a channel

diff --git a/main/channel.c b/main/channel.c
index c6df5f8..9932642 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -2640,8 +2640,13 @@ static void destroy_hooks(struct ast_channel *chan)
 }
 
 /*! \brief Hangup a channel */
-int ast_hangup(struct ast_channel *chan)
+void ast_hangup(struct ast_channel *chan)
 {
+       /* Be NULL safe for RAII_VAR() usage. */
+       if (!chan) {
+               return;
+       }
+
        ast_autoservice_stop(chan);
 
        ast_channel_lock(chan);
@@ -2669,7 +2674,7 @@ int ast_hangup(struct ast_channel *chan)
                ast_set_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE);
                destroy_hooks(chan);
                ast_channel_unlock(chan);
-               return 0;
+               return;
        }
 
        /* Mark as a zombie so a masquerade cannot be setup on this channel. */
@@ -2733,8 +2738,6 @@ int ast_hangup(struct ast_channel *chan)
        ast_cc_offer(chan);
 
        ast_channel_unref(chan);
-
-       return 0;
 }
 
 int ast_raw_answer(struct ast_channel *chan)

diff --git a/main/dial.c b/main/dial.c
index ab35373..e13805a 100644 (file)
--- a/main/dial.c
+++ b/main/dial.c
@@ -942,10 +942,8 @@ void ast_dial_hangup(struct ast_dial *dial)
 
        AST_LIST_LOCK(&dial->channels);
        AST_LIST_TRAVERSE(&dial->channels, channel, list) {
-               if (channel->owner) {
-                       ast_hangup(channel->owner);
-                       channel->owner = NULL;
-               }
+               ast_hangup(channel->owner);
+               channel->owner = NULL;
        }
        AST_LIST_UNLOCK(&dial->channels);
 
@@ -976,11 +974,11 @@ int ast_dial_destroy(struct ast_dial *dial)
                                option_types[i].disable(channel->options[i]);
                        channel->options[i] = NULL;
                }
+
                /* Hang up channel if need be */
-               if (channel->owner) {
-                       ast_hangup(channel->owner);
-                       channel->owner = NULL;
-               }
+               ast_hangup(channel->owner);
+               channel->owner = NULL;
+
                /* Free structure */
                ast_free(channel->tech);
                ast_free(channel->device);

diff --git a/main/features.c b/main/features.c
index df81513..fd72b9c 100644 (file)
--- a/main/features.c
+++ b/main/features.c
@@ -2547,9 +2547,7 @@ static int builtin_atxfer(struct ast_channel *chan, struct ast_channel *peer, st
                 * is up, hang it up as it has no one to talk to.
                 */
                ast_debug(1, "Everyone is hungup.\n");
-               if (newchan) {
-                       ast_hangup(newchan);
-               }
+               ast_hangup(newchan);
                ast_party_connected_line_free(&connected_line);
                return -1;
        }
@@ -5586,18 +5584,14 @@ AST_TEST_DEFINE(features_test)
        /* find the real channel */
        parked_chan = ast_channel_get_by_name("TestChannel1");
        if (unpark_test_channel(parked_chan, &args)) {
-               if (parked_chan) {
-                       ast_hangup(parked_chan);
-               }
+               ast_hangup(parked_chan);
                res = -1;
        }
 
 
 exit_features_test:
 
-       if (test_channel1) {
-               ast_hangup(test_channel1);
-       }
+       ast_hangup(test_channel1);
 
        force_reload_load = 1;
        ast_features_reload();

diff --git a/tests/test_app.c b/tests/test_app.c
index 0bb43aa..f47b052 100644 (file)
--- a/tests/test_app.c
+++ b/tests/test_app.c
@@ -212,18 +212,10 @@ AST_TEST_DEFINE(app_group)
        }
 
 exit_group_test:
-       if (test_channel1) {
-               ast_hangup(test_channel1);
-       }
-       if (test_channel2) {
-               ast_hangup(test_channel2);
-       }
-       if (test_channel3) {
-               ast_hangup(test_channel3);
-       }
-       if (test_channel4) {
-               ast_hangup(test_channel4);
-       }
+       ast_hangup(test_channel1);
+       ast_hangup(test_channel2);
+       ast_hangup(test_channel3);
+       ast_hangup(test_channel4);
        return res;
 }
 

diff --git a/tests/test_cdr.c b/tests/test_cdr.c
index a60f78e..670334c 100644 (file)
--- a/tests/test_cdr.c
+++ b/tests/test_cdr.c
@@ -278,11 +278,12 @@ static void clear_mock_cdr_backend(void)
        } while (0)
 
 /*! \brief Hang up a test channel safely */
-#define HANGUP_CHANNEL(channel, cause) do { \
-       ast_channel_hangupcause_set((channel), (cause)); \
-       if (!ast_hangup((channel))) { \
+#define HANGUP_CHANNEL(channel, cause) \
+       do { \
+               ast_channel_hangupcause_set((channel), (cause)); \
+               ast_hangup(channel); \
                channel = NULL; \
-       } } while (0)
+       } while (0)
 
 static enum ast_test_result_state verify_mock_cdr_record(struct ast_test *test, struct ast_cdr *expected, int record)
 {
@@ -2201,9 +2202,8 @@ AST_TEST_DEFINE(test_cdr_fields)
 
        /* Hang up and verify */
        ast_channel_hangupcause_set(chan, AST_CAUSE_NORMAL);
-       if (!ast_hangup(chan)) {
-               chan = NULL;
-       }
+       ast_hangup(chan);
+       chan = NULL;
        result = verify_mock_cdr_record(test, expected, 3);
 
        return result;
@@ -2272,9 +2272,8 @@ AST_TEST_DEFINE(test_cdr_no_reset_cdr)
        ast_test_validate(test, ast_cdr_fork(ast_channel_name(chan), &fork_options) == 0);
 
        ast_channel_hangupcause_set(chan, AST_CAUSE_NORMAL);
-       if (!ast_hangup(chan)) {
-               chan = NULL;
-       }
+       ast_hangup(chan);
+       chan = NULL;
        result = verify_mock_cdr_record(test, &expected, 1);
 
        return result;
@@ -2395,9 +2394,8 @@ AST_TEST_DEFINE(test_cdr_fork_cdr)
        ast_test_validate(test, strcmp(fork_answer_time, answer_time) != 0);
 
        ast_channel_hangupcause_set(chan, AST_CAUSE_NORMAL);
-       if (!ast_hangup(chan)) {
-               chan = NULL;
-       }
+       ast_hangup(chan);
+       chan = NULL;
        result = verify_mock_cdr_record(test, expected, 3);
 
        return result;

diff --git a/tests/test_cel.c b/tests/test_cel.c
index d785ef1..c29b2e7 100644 (file)
--- a/tests/test_cel.c
+++ b/tests/test_cel.c
@@ -139,23 +139,17 @@ static void do_sleep(void)
        } while (0)
 
 /*! \brief Hang up a test channel safely */
-#define HANGUP_CHANNEL(channel, cause, hangup_extra) do { \
-       ast_channel_hangupcause_set((channel), (cause)); \
-       ao2_ref(channel, +1); \
-       if (!ast_hangup((channel))) { \
+#define HANGUP_CHANNEL(channel, cause, hangup_extra) \
+       do { \
+               ast_channel_hangupcause_set((channel), (cause)); \
+               ao2_ref(channel, +1); \
+               ast_hangup(channel); \
                APPEND_EVENT(channel, AST_CEL_HANGUP, NULL, hangup_extra, NULL); \
                APPEND_EVENT(channel, AST_CEL_CHANNEL_END, NULL, NULL, NULL); \
                ao2_cleanup(stasis_cache_get_extended(ast_channel_topic_all_cached(), \
                        ast_channel_snapshot_type(), ast_channel_uniqueid(channel), 1)); \
                ao2_cleanup(channel); \
                channel = NULL; \
-       } else { \
-               APPEND_EVENT(channel, AST_CEL_HANGUP, NULL, hangup_extra, NULL); \
-               APPEND_EVENT(channel, AST_CEL_CHANNEL_END, NULL, NULL, NULL); \
-               ao2_cleanup(stasis_cache_get_extended(ast_channel_topic_all_cached(), \
-                       ast_channel_snapshot_type(), ast_channel_uniqueid(channel), 1)); \
-               ao2_cleanup(channel); \
-       } \
        } while (0)
 
 static int append_expected_event(

diff --git a/tests/test_stasis_endpoints.c b/tests/test_stasis_endpoints.c
index c6cafd6..9fe3ecf 100644 (file)
--- a/tests/test_stasis_endpoints.c
+++ b/tests/test_stasis_endpoints.c
@@ -45,14 +45,6 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 static const char *test_category = "/stasis/endpoints/";
 
-static void safe_channel_hangup(struct ast_channel *chan)
-{
-       if (!chan) {
-               return;
-       }
-       ast_hangup(chan);
-}
-
 /*! \brief Message matcher looking for cache update messages */
 static int cache_update(struct stasis_message *msg, const void *data) {
        struct stasis_cache_update *update;
@@ -79,7 +71,7 @@ static int cache_update(struct stasis_message *msg, const void *data) {
 AST_TEST_DEFINE(state_changes)
 {
        RAII_VAR(struct ast_endpoint *, uut, NULL, ast_endpoint_shutdown);
-       RAII_VAR(struct ast_channel *, chan, NULL, safe_channel_hangup);
+       RAII_VAR(struct ast_channel *, chan, NULL, ast_hangup);
        RAII_VAR(struct stasis_message_sink *, sink, NULL, ao2_cleanup);
        RAII_VAR(struct stasis_subscription *, sub, NULL, stasis_unsubscribe);
        struct stasis_message *msg;
@@ -135,7 +127,7 @@ AST_TEST_DEFINE(state_changes)
 AST_TEST_DEFINE(cache_clear)
 {
        RAII_VAR(struct ast_endpoint *, uut, NULL, ast_endpoint_shutdown);
-       RAII_VAR(struct ast_channel *, chan, NULL, safe_channel_hangup);
+       RAII_VAR(struct ast_channel *, chan, NULL, ast_hangup);
        RAII_VAR(struct stasis_message_sink *, sink, NULL, ao2_cleanup);
        RAII_VAR(struct stasis_subscription *, sub, NULL, stasis_unsubscribe);
        struct stasis_message *msg;
@@ -211,7 +203,7 @@ AST_TEST_DEFINE(cache_clear)
 AST_TEST_DEFINE(channel_messages)
 {
        RAII_VAR(struct ast_endpoint *, uut, NULL, ast_endpoint_shutdown);
-       RAII_VAR(struct ast_channel *, chan, NULL, safe_channel_hangup);
+       RAII_VAR(struct ast_channel *, chan, NULL, ast_hangup);
        RAII_VAR(struct stasis_message_sink *, sink, NULL, ao2_cleanup);
        RAII_VAR(struct stasis_subscription *, sub, NULL, stasis_unsubscribe);
        struct stasis_message *msg;
@@ -261,7 +253,7 @@ AST_TEST_DEFINE(channel_messages)
        actual_snapshot = stasis_message_data(msg);
        ast_test_validate(test, 1 == actual_snapshot->num_channels);
 
-       safe_channel_hangup(chan);
+       ast_hangup(chan);
        chan = NULL;
 
        actual_count = stasis_message_sink_wait_for_count(sink, 5,

diff --git a/tests/test_voicemail_api.c b/tests/test_voicemail_api.c
index 30f6f11..a0dc240 100644 (file)
--- a/tests/test_voicemail_api.c
+++ b/tests/test_voicemail_api.c
@@ -229,9 +229,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #define VM_API_PLAYBACK_MESSAGE(channel, mailbox, context, folder, message, callback_fn) do { \
        if (ast_vm_msg_play((channel), (mailbox), (context), (folder), (message), (callback_fn))) { \
                ast_test_status_update(test, "Failed nominal playback message test\n"); \
-               if (test_channel) { \
-                       ast_hangup(test_channel); \
-               } \
+               ast_hangup(test_channel); \
                VM_API_TEST_CLEANUP; \
                return AST_TEST_FAIL; \
        } } while (0)
@@ -241,9 +239,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #define VM_API_PLAYBACK_MESSAGE_OFF_NOMINAL(channel, mailbox, context, folder, message, callback_fn) do { \
        if (!ast_vm_msg_play((channel), (mailbox), (context), (folder), (message), (callback_fn))) { \
                ast_test_status_update(test, "Succeeded in playing back of message when expected result was to fail\n"); \
-               if (test_channel) { \
-                       ast_hangup(test_channel); \
-               } \
+               ast_hangup(test_channel); \
                VM_API_TEST_CLEANUP; \
                return AST_TEST_FAIL; \
        } } while (0)
@@ -1322,9 +1318,7 @@ AST_TEST_DEFINE(voicemail_api_nominal_msg_playback)
        VM_API_INT_VERIFY(test_mbox_snapshot->total_msg_num, 2);
        test_mbox_snapshot = ast_vm_mailbox_snapshot_destroy(test_mbox_snapshot);
 
-       if (test_channel) {
-               ast_hangup(test_channel);
-       }
+       ast_hangup(test_channel);
        VM_API_TEST_CLEANUP;
 
        return AST_TEST_PASS;
@@ -1382,9 +1376,7 @@ AST_TEST_DEFINE(voicemail_api_off_nominal_msg_playback)
 
        ast_test_status_update(test, "Playing back message with NULL message specifier\n");
        VM_API_PLAYBACK_MESSAGE_OFF_NOMINAL(test_channel, "test_vm_api_1234", "default", "INBOX", NULL, NULL);
-       if (test_channel) {
-               ast_hangup(test_channel);
-       }
+       ast_hangup(test_channel);
        VM_API_TEST_CLEANUP;
 
        return AST_TEST_PASS;