[Asterisk-code-review] res parking: Misc fixes. (asterisk[master])
Richard Mudgett
asteriskteam at digium.com
Fri Mar 25 18:14:22 CDT 2016
Richard Mudgett has uploaded a new change for review.
https://gerrit.asterisk.org/2474
Change subject: res_parking: Misc fixes.
......................................................................
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
---
M res/parking/parking_applications.c
M res/parking/parking_bridge.c
M res/parking/parking_bridge_features.c
M res/parking/parking_manager.c
M res/res_parking.c
5 files changed, 38 insertions(+), 52 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/2474/1
diff --git a/res/parking/parking_applications.c b/res/parking/parking_applications.c
index 0a0ea3c..bd72b85 100644
--- a/res/parking/parking_applications.c
+++ b/res/parking/parking_applications.c
@@ -339,16 +339,17 @@
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 @@
/* 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 @@
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 @@
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 *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 @@
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 @@
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 @@
}
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 @@
}
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 4f7198b..e61486e 100644
--- a/res/parking/parking_bridge.c
+++ b/res/parking/parking_bridge.c
@@ -167,7 +167,7 @@
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 @@
* 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 @@
/* 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 4cb87c8..70c2fc6 100644
--- a/res/parking/parking_bridge_features.c
+++ b/res/parking/parking_bridge_features.c
@@ -238,6 +238,7 @@
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);
@@ -251,8 +252,6 @@
ast_hangup(parkee);
return NULL;
}
-
- ast_bridge_set_transfer_variables(parkee_side_2, ast_channel_name(parker), 0);
ast_channel_unref(parkee_side_2);
diff --git a/res/parking/parking_manager.c b/res/parking/parking_manager.c
index a560151..89f553d 100644
--- a/res/parking/parking_manager.c
+++ b/res/parking/parking_manager.c
@@ -537,12 +537,12 @@
}
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 @@
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 02740da..26f08e4 100644
--- a/res/res_parking.c
+++ b/res/res_parking.c
@@ -738,31 +738,21 @@
}
/* 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 @@
}
}
- if (ast_unlock_context(lot_context)) {
- ast_assert(0);
- }
+ ast_unlock_context(lot_context);
return 0;
}
--
To view, visit https://gerrit.asterisk.org/2474
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib64945bc285acb05a306dc12e6f16854898915ca
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list