[Asterisk-code-review] res fax.c: Remove redundant locking. (asterisk[11])

[Richard Mudgett]

Richard Mudgett has uploaded a new change for review.

  https://gerrit.asterisk.org/3699

Change subject: res_fax.c: Remove redundant locking.
......................................................................

res_fax.c: Remove redundant locking.

When FAX was developed, apparently the faxregistry.container used to be a
linked list that was converted to an ao2 container.  Some of the
replacement ao2 container operations still had explicit lock/unlocks
around them.

Two off nominal code paths in res_fax.c unlock the channel even though the
routine did not lock the channel and other code paths in the routine do
not unlock the channel.

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

Backported from v13 revision fa80d9658df8aff71a8975ab7d1fe477ea3f99b9.

Change-Id: I59a1b95a91dac8375e0db568ddb217bcd6d7d2fa
---
M res/res_fax.c
1 file changed, 4 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/99/3699/1

diff --git a/res/res_fax.c b/res/res_fax.c
index f4deca4..5f660fe 100644
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -1370,21 +1370,15 @@
 		ast_format_copy(&orig_write_format, ast_channel_writeformat(chan));
 		if (ast_set_write_format_by_id(chan, AST_FORMAT_SLINEAR) < 0) {
 			ast_log(LOG_ERROR, "channel '%s' failed to set write format to signed linear'.\n", ast_channel_name(chan));
- 			ao2_lock(faxregistry.container);
- 			ao2_unlink(faxregistry.container, fax);
- 			ao2_unlock(faxregistry.container);
- 			ao2_ref(fax, -1);
-			ast_channel_unlock(chan);
+			ao2_unlink(faxregistry.container, fax);
+			ao2_ref(fax, -1);
 			return -1;
 		}
 		ast_format_copy(&orig_read_format, ast_channel_readformat(chan));
 		if (ast_set_read_format_by_id(chan, AST_FORMAT_SLINEAR) < 0) {
 			ast_log(LOG_ERROR, "channel '%s' failed to set read format to signed linear.\n", ast_channel_name(chan));
- 			ao2_lock(faxregistry.container);
- 			ao2_unlink(faxregistry.container, fax);
- 			ao2_unlock(faxregistry.container);
- 			ao2_ref(fax, -1);
-			ast_channel_unlock(chan);
+			ao2_unlink(faxregistry.container, fax);
+			ao2_ref(fax, -1);
 			return -1;
 		}
 		if (fax->smoother) {
@@ -1560,9 +1554,7 @@
 	}
 
 	if (fax) {
-		ao2_lock(faxregistry.container);
 		ao2_unlink(faxregistry.container, fax);
-		ao2_unlock(faxregistry.container);
 		ao2_ref(fax, -1);
 	}
 
@@ -2508,18 +2500,14 @@
 static void destroy_v21_sessions(struct fax_gateway *gateway)
 {
 	if (gateway->chan_v21_session) {
-		ao2_lock(faxregistry.container);
 		ao2_unlink(faxregistry.container, gateway->chan_v21_session);
-		ao2_unlock(faxregistry.container);
 
 		ao2_ref(gateway->chan_v21_session, -1);
 		gateway->chan_v21_session = NULL;
 	}
 
 	if (gateway->peer_v21_session) {
-		ao2_lock(faxregistry.container);
 		ao2_unlink(faxregistry.container, gateway->peer_v21_session);
-		ao2_unlock(faxregistry.container);
 
 		ao2_ref(gateway->peer_v21_session, -1);
 		gateway->peer_v21_session = NULL;
@@ -2537,9 +2525,7 @@
 		fax_session_release(gateway->s, gateway->token);
 		gateway->token = NULL;
 
-		ao2_lock(faxregistry.container);
 		ao2_unlink(faxregistry.container, gateway->s);
-		ao2_unlock(faxregistry.container);
 
 		ao2_ref(gateway->s, -1);
 		gateway->s = NULL;

-- 
To view, visit https://gerrit.asterisk.org/3699
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I59a1b95a91dac8375e0db568ddb217bcd6d7d2fa
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>