Misc format capability fixes.
authorRichard Mudgett <rmudgett@digium.com>
Mon, 31 Oct 2011 17:51:22 +0000 (17:51 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Mon, 31 Oct 2011 17:51:22 +0000 (17:51 +0000)
* 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
main/format_cap.c

index 1c60ab7..a365ab0 100644 (file)
@@ -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);
index b632a7b..a3e5131 100644 (file)
@@ -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,