[svn-commits] rmudgett: trunk r342825 - in /trunk: ./ main/format.c main/format_cap.c

[SVN commits to the Digium repositories]

Author: rmudgett
Date: Mon Oct 31 12:51:22 2011
New Revision: 342825

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=342825
Log:
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

Modified:
    trunk/   (props changed)
    trunk/main/format.c
    trunk/main/format_cap.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.

Modified: trunk/main/format.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/format.c?view=diff&rev=342825&r1=342824&r2=342825
==============================================================================
--- trunk/main/format.c (original)
+++ trunk/main/format.c Mon Oct 31 12:51:22 2011
@@ -106,7 +106,7 @@
 	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 @@
 	};
 
 	ast_rwlock_rdlock(&ilock);
-	if (!(wrapper = ao2_find(interfaces, &tmp_wrapper, (OBJ_POINTER | OBJ_NOLOCK)))) {
-		ast_rwlock_unlock(&ilock);
+	wrapper = ao2_find(interfaces, &tmp_wrapper, (OBJ_POINTER | OBJ_NOLOCK));
+	ast_rwlock_unlock(&ilock);
+
+	return wrapper;
+}
+
+static int has_interface(const struct ast_format *format)
+{
+	struct interface_ao2_wrapper *wrapper;
+
+	wrapper = find_interface(format);
+	if (!wrapper) {
 		return 0;
 	}
-	ast_rwlock_unlock(&ilock);
 	ao2_ref(wrapper, -1);
 	return 1;
-}
-
-static struct interface_ao2_wrapper *find_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;
-	}
-	ast_rwlock_unlock(&ilock);
-
-	return wrapper;
 }
 
 /*! \internal
@@ -1050,7 +1042,7 @@
 	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 @@
 	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 @@
 	}
 
 	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 @@
 		.id = interface->id,
 	};
 
+	/*
+	 * 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*/
-	ast_rwlock_wrlock(&ilock);
 	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 @@
 	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);

Modified: trunk/main/format_cap.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/format_cap.c?view=diff&rev=342825&r1=342824&r2=342825
==============================================================================
--- trunk/main/format_cap.c (original)
+++ trunk/main/format_cap.c Mon Oct 31 12:51:22 2011
@@ -470,7 +470,7 @@
 	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,