From a4189763ab562ee2e8e79fda60b213ee90aab6f1 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Fri, 18 Mar 2016 14:01:02 -0500 Subject: res_parking: Misc fixes. res/parking/parking_applications.c: * Add malloc fail checks in setup_park_common_datastore(). * Fix playing parking failed announcement to only happen on non-blind transfers in park_app_exec(). It could never go out before because a test was provedly always false. res/parking/parking_bridge.c: * Fix NULL tolerance in generate_parked_user() because bridge_parking_push() can theoretically pass a NULL parker channel if the parker channel went away for some reason. * Clarify some weird code dealing with blind_transfer in bridge_parking_push(). res/parking/parking_bridge_features.c: * Made park_local_transfer() set BLINDTRANSFER on the Local;1 channel which will be bulk copied to the Local;2 channel on the subsequent ast_call(). The additional advantage is if the parker channel has the BLINDTRANSFER and ATTENDEDTRANSFER variables set they are now guaranteed to be overridden. res/parking/parking_manager.c: * Fix AMI Park action input range checking of the Timeout header in manager_park(). * Reduced locking scope to where needed in manager_park(). res/res_parking.c: * Fix some off nominal missing unlocks by eliminating the returns. Change-Id: Ib64945bc285acb05a306dc12e6f16854898915ca --- res/parking/parking_applications.c | 40 ++++++++++++++++++----------------- res/parking/parking_bridge.c | 17 ++++++--------- res/parking/parking_bridge_features.c | 3 +-- res/parking/parking_manager.c | 8 +++---- res/res_parking.c | 22 +++++-------------- 5 files changed, 38 insertions(+), 52 deletions(-) diff --git a/res/parking/parking_applications.c b/res/parking/parking_applications.c index d8cda6f50..1bdb50fab 100644 --- a/res/parking/parking_applications.c +++ b/res/parking/parking_applications.c @@ -339,16 +339,17 @@ static int setup_park_common_datastore(struct ast_channel *parkee, const char *p ast_datastore_free(datastore); return -1; } + datastore->data = park_datastore; - if (parker_uuid) { - park_datastore->parker_uuid = ast_strdup(parker_uuid); + park_datastore->parker_uuid = ast_strdup(parker_uuid); + if (!park_datastore->parker_uuid) { + ast_datastore_free(datastore); + return -1; } ast_channel_lock(parkee); - attended_transfer = pbx_builtin_getvar_helper(parkee, "ATTENDEDTRANSFER"); blind_transfer = pbx_builtin_getvar_helper(parkee, "BLINDTRANSFER"); - if (!ast_strlen_zero(attended_transfer)) { parker_dial_string = ast_strdupa(attended_transfer); } else if (!ast_strlen_zero(blind_transfer)) { @@ -356,7 +357,6 @@ static int setup_park_common_datastore(struct ast_channel *parkee, const char *p /* Ensure that attended_transfer is NULL and not an empty string. */ attended_transfer = NULL; } - ast_channel_unlock(parkee); if (!ast_strlen_zero(parker_dial_string)) { @@ -365,6 +365,10 @@ static int setup_park_common_datastore(struct ast_channel *parkee, const char *p parker_dial_string, attended_transfer ? "ATTENDEDTRANSFER" : "BLINDTRANSFER"); park_datastore->parker_dial_string = ast_strdup(parker_dial_string); + if (!park_datastore->parker_dial_string) { + ast_datastore_free(datastore); + return -1; + } } park_datastore->randomize = randomize; @@ -373,10 +377,13 @@ static int setup_park_common_datastore(struct ast_channel *parkee, const char *p if (comeback_override) { park_datastore->comeback_override = ast_strdup(comeback_override); + if (!park_datastore->comeback_override) { + ast_datastore_free(datastore); + return -1; + } } - datastore->data = park_datastore; ast_channel_lock(parkee); ast_channel_datastore_add(parkee, datastore); ast_channel_unlock(parkee); @@ -391,23 +398,23 @@ struct park_common_datastore *get_park_common_datastore_copy(struct ast_channel struct park_common_datastore *data_copy; SCOPED_CHANNELLOCK(lock, parkee); + if (!(datastore = ast_channel_datastore_find(parkee, &park_common_info, NULL))) { return NULL; } data = datastore->data; - if (!data) { - /* This data should always be populated if this datastore was appended to the channel */ - ast_assert(0); - } + /* This data should always be populated if this datastore was appended to the channel */ + ast_assert(data != NULL); data_copy = ast_calloc(1, sizeof(*data_copy)); if (!data_copy) { return NULL; } - if (!(data_copy->parker_uuid = ast_strdup(data->parker_uuid))) { + data_copy->parker_uuid = ast_strdup(data->parker_uuid); + if (!data_copy->parker_uuid) { park_common_datastore_free(data_copy); return NULL; } @@ -457,7 +464,6 @@ struct ast_bridge *park_common_setup(struct ast_channel *parkee, struct ast_chan if (!lot) { lot = parking_create_dynamic_lot(lot_name, parkee); } - if (!lot) { ast_log(LOG_ERROR, "Could not find parking lot: '%s'\n", lot_name); return NULL; @@ -504,7 +510,7 @@ static int park_app_exec(struct ast_channel *chan, const char *data) struct ast_bridge_features chan_features; int res; int silence_announcements = 0; - const char *transferer; + int blind_transfer; /* Answer the channel if needed */ if (ast_channel_state(chan) != AST_STATE_UP) { @@ -512,15 +518,12 @@ static int park_app_exec(struct ast_channel *chan, const char *data) } ast_channel_lock(chan); - if (!(transferer = pbx_builtin_getvar_helper(chan, "ATTENDEDTRANSFER"))) { - transferer = pbx_builtin_getvar_helper(chan, "BLINDTRANSFER"); - } - transferer = ast_strdupa(S_OR(transferer, "")); + blind_transfer = !ast_strlen_zero(pbx_builtin_getvar_helper(chan, "BLINDTRANSFER")); ast_channel_unlock(chan); /* Handle the common parking setup stuff */ if (!(parking_bridge = park_application_setup(chan, NULL, data, &silence_announcements))) { - if (!silence_announcements && !transferer) { + if (!silence_announcements && !blind_transfer) { ast_stream_and_wait(chan, "pbx-parkingfailed", ""); } publish_parked_call_failure(chan); @@ -593,7 +596,6 @@ static int parked_call_app_exec(struct ast_channel *chan, const char *data) } lot = parking_lot_find_by_name(lot_name); - if (!lot) { ast_log(LOG_ERROR, "Could not find the requested parking lot\n"); ast_stream_and_wait(chan, "pbx-invalidpark", ""); diff --git a/res/parking/parking_bridge.c b/res/parking/parking_bridge.c index 4f7198b4f..e61486e3c 100644 --- a/res/parking/parking_bridge.c +++ b/res/parking/parking_bridge.c @@ -167,7 +167,7 @@ static struct parked_user *generate_parked_user(struct parking_lot *lot, struct if (parker_dial_string) { new_parked_user->parker_dial_string = ast_strdup(parker_dial_string); } else { - if (parked_user_set_parker_dial_string(new_parked_user, parker)) { + if (!parker || parked_user_set_parker_dial_string(new_parked_user, parker)) { ao2_ref(new_parked_user, -1); ao2_unlock(lot); return NULL; @@ -269,14 +269,11 @@ static int bridge_parking_push(struct ast_bridge_parking *self, struct ast_bridg * the park application. It's possible that the channel that transferred it is still alive (particularly * when a multichannel bridge is parked), so try to get the real parker if possible. */ ast_channel_lock(bridge_channel->chan); - blind_transfer = S_OR(pbx_builtin_getvar_helper(bridge_channel->chan, "BLINDTRANSFER"), - ast_channel_name(bridge_channel->chan)); - if (blind_transfer) { - blind_transfer = ast_strdupa(blind_transfer); - } + blind_transfer = pbx_builtin_getvar_helper(bridge_channel->chan, "BLINDTRANSFER"); + blind_transfer = ast_strdupa(S_OR(blind_transfer, "")); ast_channel_unlock(bridge_channel->chan); - - if (parker == bridge_channel->chan) { + if ((!parker || parker == bridge_channel->chan) + && !ast_strlen_zero(blind_transfer)) { struct ast_channel *real_parker = ast_channel_get_by_name(blind_transfer); if (real_parker) { @@ -300,8 +297,8 @@ static int bridge_parking_push(struct ast_bridge_parking *self, struct ast_bridg /* Generate ParkedCall Stasis Message */ publish_parked_call(pu, PARKED_CALL); - /* If the parkee and the parker are the same and silence_announce isn't set, play the announcement to the parkee */ - if (!strcmp(blind_transfer, ast_channel_name(bridge_channel->chan)) && !park_datastore->silence_announce) { + /* If not a blind transfer and silence_announce isn't set, play the announcement to the parkee */ + if (ast_strlen_zero(blind_transfer) && !park_datastore->silence_announce) { char saynum_buf[16]; snprintf(saynum_buf, sizeof(saynum_buf), "%d %d", 0, pu->parking_space); diff --git a/res/parking/parking_bridge_features.c b/res/parking/parking_bridge_features.c index a21be9068..ab0ea6f44 100644 --- a/res/parking/parking_bridge_features.c +++ b/res/parking/parking_bridge_features.c @@ -238,6 +238,7 @@ static struct ast_channel *park_local_transfer(struct ast_channel *parker, const ast_channel_req_accountcodes(parkee, parker, AST_CHANNEL_REQUESTOR_REPLACEMENT); ast_connected_line_copy_from_caller(ast_channel_connected(parkee), ast_channel_caller(parker)); ast_channel_inherit_variables(parker, parkee); + ast_bridge_set_transfer_variables(parkee, ast_channel_name(parker), 0); ast_channel_datastore_inherit(parker, parkee); ast_channel_unlock(parker); @@ -252,8 +253,6 @@ static struct ast_channel *park_local_transfer(struct ast_channel *parker, const return NULL; } - ast_bridge_set_transfer_variables(parkee_side_2, ast_channel_name(parker), 0); - ast_channel_unref(parkee_side_2); /* Since the above worked fine now we actually call it and return the channel */ diff --git a/res/parking/parking_manager.c b/res/parking/parking_manager.c index 435200b1e..ed28164fe 100644 --- a/res/parking/parking_manager.c +++ b/res/parking/parking_manager.c @@ -537,12 +537,12 @@ static int manager_park(struct mansession *s, const struct message *m) } if (!ast_strlen_zero(timeout)) { - if (sscanf(timeout, "%30d", &timeout_override) != 1 || timeout < 0) { + if (sscanf(timeout, "%30d", &timeout_override) != 1 || timeout_override < 0) { astman_send_error(s, m, "Invalid Timeout value."); return 0; } - if (timeout_override > 0) { + if (timeout_override) { /* If greater than zero, convert to seconds for internal use. Must be >= 1 second. */ timeout_override = MAX(1, timeout_override / 1000); } @@ -554,11 +554,11 @@ static int manager_park(struct mansession *s, const struct message *m) return 0; } - ast_channel_lock(chan); if (!ast_strlen_zero(timeout_channel)) { + ast_channel_lock(chan); ast_bridge_set_transfer_variables(chan, timeout_channel, 0); + ast_channel_unlock(chan); } - ast_channel_unlock(chan); parker_chan = ast_channel_bridge_peer(chan); if (!parker_chan || strcmp(ast_channel_name(parker_chan), timeout_channel)) { diff --git a/res/res_parking.c b/res/res_parking.c index 3edbd4663..6696980c7 100644 --- a/res/res_parking.c +++ b/res/res_parking.c @@ -738,31 +738,21 @@ int parking_lot_cfg_create_extensions(struct parking_lot_cfg *lot_cfg) } /* We need the contexts list locked to safely be able to both read and lock the specific context within */ - if (ast_wrlock_contexts()) { - ast_log(LOG_ERROR, "Failed to lock the contexts list.\n"); - return -1; - } + ast_wrlock_contexts(); if (!(lot_context = ast_context_find_or_create(NULL, NULL, lot_cfg->parking_con, parkext_registrar_pointer))) { ast_log(LOG_ERROR, "Parking lot '%s' -- Needs a context '%s' which does not exist and Asterisk was unable to create\n", lot_cfg->name, lot_cfg->parking_con); - if (ast_unlock_contexts()) { - ast_assert(0); - } + ast_unlock_contexts(); return -1; } /* Once we know what context we will be modifying, we need to write lock it because we will be reading extensions * and we don't want something else to destroy them while we are looking at them. */ - if (ast_wrlock_context(lot_context)) { - ast_log(LOG_ERROR, "failed to obtain write lock on context\n"); - return -1; - } + ast_wrlock_context(lot_context); - if (ast_unlock_contexts()) { - ast_assert(0); - } + ast_unlock_contexts(); /* Handle generation/confirmation for the Park extension */ if ((existing_exten = pbx_find_extension(NULL, NULL, &find_info, lot_cfg->parking_con, lot_cfg->parkext, 1, NULL, NULL, E_MATCH))) { @@ -830,9 +820,7 @@ int parking_lot_cfg_create_extensions(struct parking_lot_cfg *lot_cfg) } } - if (ast_unlock_context(lot_context)) { - ast_assert(0); - } + ast_unlock_context(lot_context); return 0; } -- cgit v1.2.3