[asterisk-commits] seanbright: trunk r239113 - /trunk/main/astobj2.c

[SVN commits to the Asterisk project]

Author: seanbright
Date: Mon Jan 11 10:40:23 2010
New Revision: 239113

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=239113
Log:
Fix ao2_callback when both OBJ_MULTIPLE and OBJ_NODATA are passed.

There is an issue which only affects trunk and the new ao2_callback OBJ_MULTIPLE
implementation.  When both OBJ_MULTIPLE and OBJ_NODATA are passed, only the first
object is visited, regardless of what is returned by the specified callback. This
causes a problem when we are clearing a container, i.e.:

    ao2_callback(container, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL, NULL);

Only unlinks the first object.  This patch resolves this.

(closes issue #16564)
Reported by: pj
Patches:
      issue16564_20100111.diff uploaded by seanbright (license 71)
Tested by: pj, seanbright

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

Modified:
    trunk/main/astobj2.c

Modified: trunk/main/astobj2.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/astobj2.c?view=diff&rev=239113&r1=239112&r2=239113
==============================================================================
--- trunk/main/astobj2.c (original)
+++ trunk/main/astobj2.c Mon Jan 11 10:40:23 2010
@@ -593,6 +593,8 @@
 	return CMP_MATCH;
 }
 
+#define USE_CONTAINER(x) (((x) & (OBJ_MULTIPLE | OBJ_NODATA)) == OBJ_MULTIPLE)
+
 /*!
  * Browse the container using different stategies accoding the flags.
  * \return Is a pointer to an object or to a list of object if OBJ_MULTIPLE is 
@@ -615,7 +617,7 @@
 	if (INTERNAL_OBJ(c) == NULL)	/* safety check on the argument */
 		return NULL;
 
-	if ((flags & (OBJ_MULTIPLE | OBJ_NODATA)) == OBJ_MULTIPLE) {
+	if (USE_CONTAINER(flags)) {
 		/* we need to return an ao2_iterator with the results,
 		 * as there could be more than one. the iterator will
 		 * hold the only reference to a container that has all the
@@ -708,7 +710,7 @@
 			/* if we are in OBJ_MULTIPLE mode, link the object into the
 			 * container that will hold the results
 			 */
-			if (ret && (multi_container != NULL)) {
+			if (ret && USE_CONTAINER(flags)) {
 				__ao2_link(multi_container, ret);
 				ret = NULL;
 			}
@@ -731,9 +733,10 @@
 				ast_free(cur);	/* free the link record */
 			}
 
-			if ((match & CMP_STOP) || (multi_container == NULL)) {
-				/* We found the only match we need */
-				i = last;	/* force exit from outer loop */
+			if ((match & CMP_STOP) || USE_CONTAINER(flags)) {
+				/* We found our only (or last) match, so force an exit from
+				   the outside loop. */
+				i = last;
 				break;
 			}
 		}
@@ -750,7 +753,7 @@
 		}
 	}
 	ao2_unlock(c);
-	if (multi_container != NULL) {
+	if (USE_CONTAINER(flags)) {
 		*multi_iterator = ao2_iterator_init(multi_container,
 						    AO2_ITERATOR_DONTLOCK | AO2_ITERATOR_UNLINK | AO2_ITERATOR_MALLOCD);
 		ao2_ref(multi_container, -1);