From ed1e02a4d35a66697c8c73432c493b155d638c1b Mon Sep 17 00:00:00 2001
From: Richard Mudgett <rmudgett@digium.com>
Date: Mon, 31 Oct 2011 17:51:22 +0000
Subject: Misc format capability fixes.

* Fixed typo in format_cap.c:joint_copy_helper() using the wrong variable.

* Fix potential race between checking if an interface exists and adding it
to the container in format.c:ast_format_attr_reg_interface().

* Fixed double rwlock destroy in format.c:ast_format_attr_init() error
exit path.

* Simplified format.c:find_interface() and format.c:has_interface().
........

Merged revisions 342824 from http://svn.asterisk.org/svn/asterisk/branches/10


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@342825 65c4cc65-6c06-0410-ace0-fbb531ad65f3
---
 main/format.c     | 48 ++++++++++++++++++++++--------------------------
 main/format_cap.c |  2 +-
 2 files changed, 23 insertions(+), 27 deletions(-)

(limited to 'main')

diff --git a/main/format.c b/main/format.c
index 1c60ab770..a365ab0d5 100644
--- a/main/format.c
+++ b/main/format.c
@@ -106,7 +106,7 @@ int ast_format_get_video_mark(const struct ast_format *format)
 	return format->fattr.rtp_marker_bit;
 }
 
-static int has_interface(const struct ast_format *format)
+static struct interface_ao2_wrapper *find_interface(const struct ast_format *format)
 {
 	struct interface_ao2_wrapper *wrapper;
 	struct interface_ao2_wrapper tmp_wrapper = {
@@ -114,30 +114,22 @@ static int has_interface(const struct ast_format *format)
 	};
 
 	ast_rwlock_rdlock(&ilock);
-	if (!(wrapper = ao2_find(interfaces, &tmp_wrapper, (OBJ_POINTER | OBJ_NOLOCK)))) {
-		ast_rwlock_unlock(&ilock);
-		return 0;
-	}
+	wrapper = ao2_find(interfaces, &tmp_wrapper, (OBJ_POINTER | OBJ_NOLOCK));
 	ast_rwlock_unlock(&ilock);
-	ao2_ref(wrapper, -1);
-	return 1;
+
+	return wrapper;
 }
 
-static struct interface_ao2_wrapper *find_interface(const struct ast_format *format)
+static int has_interface(const struct ast_format *format)
 {
 	struct interface_ao2_wrapper *wrapper;
-	struct interface_ao2_wrapper tmp_wrapper = {
-		.id = format->id,
-	};
 
-	ast_rwlock_rdlock(&ilock);
-	if (!(wrapper = ao2_find(interfaces, &tmp_wrapper, (OBJ_POINTER | OBJ_NOLOCK)))) {
-		ast_rwlock_unlock(&ilock);
-		return NULL;
+	wrapper = find_interface(format);
+	if (!wrapper) {
+		return 0;
 	}
-	ast_rwlock_unlock(&ilock);
-
-	return wrapper;
+	ao2_ref(wrapper, -1);
+	return 1;
 }
 
 /*! \internal
@@ -1050,7 +1042,7 @@ static int format_list_init(void)
 	return 0;
 }
 
-int ast_format_list_init()
+int ast_format_list_init(void)
 {
 	if (ast_rwlock_init(&format_list_array_lock)) {
 		return -1;
@@ -1073,7 +1065,7 @@ init_list_cleanup:
 	return -1;
 }
 
-int ast_format_attr_init()
+int ast_format_attr_init(void)
 {
 	ast_cli_register_multiple(my_clis, ARRAY_LEN(my_clis));
 	if (ast_rwlock_init(&ilock)) {
@@ -1081,7 +1073,6 @@ int ast_format_attr_init()
 	}
 
 	if (!(interfaces = ao2_container_alloc(283, interface_hash_cb, interface_cmp_cb))) {
-		ast_rwlock_destroy(&ilock);
 		goto init_cleanup;
 	}
 	return 0;
@@ -1316,17 +1307,23 @@ int ast_format_attr_reg_interface(const struct ast_format_attr_interface *interf
 		.id = interface->id,
 	};
 
-	/* check for duplicates first*/
+	/*
+	 * Grab the write lock before checking for duplicates in
+	 * anticipation of adding a new interface and to prevent a
+	 * duplicate from sneaking in between the check and add.
+	 */
 	ast_rwlock_wrlock(&ilock);
+
+	/* check for duplicates first*/
 	if ((wrapper = ao2_find(interfaces, &tmp_wrapper, (OBJ_POINTER | OBJ_NOLOCK)))) {
 		ast_rwlock_unlock(&ilock);
 		ast_log(LOG_WARNING, "Can not register attribute interface for format id %d, interface already exists.\n", interface->id);
 		ao2_ref(wrapper, -1);
 		return -1;
 	}
-	ast_rwlock_unlock(&ilock);
 
 	if (!(wrapper = ao2_alloc(sizeof(*wrapper), interface_destroy_cb))) {
+		ast_rwlock_unlock(&ilock);
 		return -1;
 	}
 
@@ -1334,9 +1331,8 @@ int ast_format_attr_reg_interface(const struct ast_format_attr_interface *interf
 	wrapper->id = interface->id;
 	ast_rwlock_init(&wrapper->wraplock);
 
-	/* use the write lock whenever the interface container is modified */
-	ast_rwlock_wrlock(&ilock);
-	ao2_link(interfaces, wrapper);
+	/* The write lock is already held. */
+	ao2_link_nolock(interfaces, wrapper);
 	ast_rwlock_unlock(&ilock);
 
 	ao2_ref(wrapper, -1);
diff --git a/main/format_cap.c b/main/format_cap.c
index b632a7b1c..a3e513160 100644
--- a/main/format_cap.c
+++ b/main/format_cap.c
@@ -470,7 +470,7 @@ static int joint_copy_helper(const struct ast_format_cap *cap1, const struct ast
 	if (!append) {
 		ast_format_cap_remove_all(result);
 	}
-	it = ao2_iterator_init(cap1->formats, cap2->nolock ? AO2_ITERATOR_DONTLOCK : 0);
+	it = ao2_iterator_init(cap1->formats, cap1->nolock ? AO2_ITERATOR_DONTLOCK : 0);
 	while ((tmp = ao2_iterator_next(&it))) {
 		data.format = tmp;
 		ao2_callback(cap2->formats,
-- 
cgit v1.2.3