Finish implementaton of astobj2 OBJ_MULTIPLE, and convert ast_channel_iterator to use it.

This patch finishes the implementation of OBJ_MULTIPLE in astobj2 (the
case where multiple results need to be returned; OBJ_NODATA mode
already was supported). In addition, it converts ast_channel_iterators
(only the targeted versions, not the ones that iterate over all
channels) to use this method.

During this work, I removed the 'ao2_flags' arguments to the
ast_channel_iterator constructor functions; there were no uses of that
argument yet, there is only one possible flag to pass, and it made the
iterators less 'opaque'. If at some point in the future someone really
needs an ast_channel_iterator that does not lock the container, we can
provide constructor(s) for that purpose.

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



git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@225244 65c4cc65-6c06-0410-ace0-fbb531ad65f3

4 files changed, 126 insertions, 135 deletions

diff --git a/main/astobj2.c b/main/astobj2.c
index 01fd4b17f..cecef7c66 100644
--- a/main/astobj2.c
+++ b/main/astobj2.c
@@ -292,7 +292,7 @@ static int internal_ao2_ref(void *user_data, const int delta)
 		 * first word of the user-data, which we make sure is always
 		 * allocated. */
 		memset(obj, '\0', sizeof(struct astobj2 *) + sizeof(void *) );
-		free(obj);
+		ast_free(obj);
 	}
 
 	return ret;
@@ -609,13 +609,27 @@ static void *internal_ao2_callback(struct ao2_container *c,
 	void *ret = NULL;
 	ao2_callback_fn *cb_default = NULL;
 	ao2_callback_data_fn *cb_withdata = NULL;
+	struct ao2_container *multi_container = NULL;
+	struct ao2_iterator *multi_iterator = NULL;
 
 	if (INTERNAL_OBJ(c) == NULL)	/* safety check on the argument */
 		return NULL;
 
 	if ((flags & (OBJ_MULTIPLE | OBJ_NODATA)) == OBJ_MULTIPLE) {
-		ast_log(LOG_WARNING, "multiple data return not implemented yet (flags %x)\n", flags);
-		return NULL;
+		/* 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
+		 * matching objects linked into it, so when the iterator
+		 * is destroyed, the container will be automatically
+		 * destroyed as well.
+		 */
+		if (!(multi_container = __ao2_container_alloc(1, NULL, NULL))) {
+			return NULL;
+		}
+		if (!(multi_iterator = ast_calloc(1, sizeof(*multi_iterator)))) {
+			ao2_ref(multi_container, -1);
+			return NULL;
+		}
 	}
 
 	/* override the match function if necessary */
@@ -627,7 +641,7 @@ static void *internal_ao2_callback(struct ao2_container *c,
 		}
 	} else {
 		/* We do this here to avoid the per object casting penalty (even though
-		   that is probably optimized away anyway. */
+		   that is probably optimized away anyway). */
 		if (type == WITH_DATA) {
 			cb_withdata = cb_fn;
 		} else {
@@ -678,49 +692,51 @@ static void *internal_ao2_callback(struct ao2_container *c,
 				i = last;
 				break;
 			}
+
 			/* we have a match (CMP_MATCH) here */
 			if (!(flags & OBJ_NODATA)) {	/* if must return the object, record the value */
 				/* it is important to handle this case before the unlink */
 				ret = EXTERNAL_OBJ(cur->astobj);
-				if (tag)
-					__ao2_ref_debug(ret, 1, tag, file, line, funcname);
-				else
-					__ao2_ref(ret, 1);
+				if (!(flags & (OBJ_UNLINK | OBJ_MULTIPLE))) {
+					if (tag)
+						__ao2_ref_debug(ret, 1, tag, file, line, funcname);
+					else
+						__ao2_ref(ret, 1);
+				}
 			}
 
-			if (flags & OBJ_UNLINK) {	/* must unlink */
-				struct bucket_list *x = cur;
+			/* if we are in OBJ_MULTIPLE mode, link the object into the
+			 * container that will hold the results
+			 */
+			if (ret && (multi_container != NULL)) {
+				__ao2_link(multi_container, ret);
+				ret = NULL;
+			}
 
+			if (flags & OBJ_UNLINK) {	/* must unlink */
 				/* we are going to modify the container, so update version */
 				ast_atomic_fetchadd_int(&c->version, 1);
 				AST_LIST_REMOVE_CURRENT(entry);
-				/* update number of elements and version */
+				/* update number of elements */
 				ast_atomic_fetchadd_int(&c->elements, -1);
-				if (tag)
-					__ao2_ref_debug(EXTERNAL_OBJ(x->astobj), -1, tag, file, line, funcname);
-				else
-					__ao2_ref(EXTERNAL_OBJ(x->astobj), -1);
-				free(x);	/* free the link record */
+				if (!(flags & OBJ_NODATA)) {
+					if (tag)
+						__ao2_ref_debug(EXTERNAL_OBJ(cur->astobj), -1, tag, file, line, funcname);
+					else
+						__ao2_ref(EXTERNAL_OBJ(cur->astobj), -1);
+				}
+				ast_free(cur);	/* free the link record */
 			}
 
-			if ((match & CMP_STOP) || (flags & OBJ_MULTIPLE) == 0) {
+			if ((match & CMP_STOP) || (multi_container == NULL)) {
 				/* We found the only match we need */
 				i = last;	/* force exit from outer loop */
 				break;
 			}
-			if (!(flags & OBJ_NODATA)) {
-#if 0	/* XXX to be completed */
-				/*
-				 * This is the multiple-return case. We need to link
-				 * the object in a list. The refcount is already increased.
-				 */
-#endif
-			}
 		}
 		AST_LIST_TRAVERSE_SAFE_END;
 
 		if (ret) {
-			/* This assumes OBJ_MULTIPLE with !OBJ_NODATA is still not implemented */
 			break;
 		}
 
@@ -731,7 +747,14 @@ static void *internal_ao2_callback(struct ao2_container *c,
 		}
 	}
 	ao2_unlock(c);
-	return ret;
+	if (multi_container != NULL) {
+		*multi_iterator = ao2_iterator_init(multi_container,
+						    AO2_ITERATOR_DONTLOCK | AO2_ITERATOR_UNLINK | AO2_ITERATOR_MALLOCD);
+		ao2_ref(multi_container, -1);
+		return multi_iterator;
+	} else {
+		return ret;
+	}
 }
 
 void *__ao2_callback_debug(struct ao2_container *c,
@@ -796,7 +819,11 @@ struct ao2_iterator ao2_iterator_init(struct ao2_container *c, int flags)
 void ao2_iterator_destroy(struct ao2_iterator *i)
 {
 	ao2_ref(i->c, -1);
-	i->c = NULL;
+	if (i->flags & AO2_ITERATOR_MALLOCD) {
+		ast_free(i);
+	} else {
+		i->c = NULL;
+	}
 }
 
 /*
@@ -819,8 +846,8 @@ static void *internal_ao2_iterator_next(struct ao2_iterator *a, struct bucket_li
 	/* optimization. If the container is unchanged and
 	 * we have a pointer, try follow it
 	 */
-	if (a->c->version == a->c_version && (p = a->obj) ) {
-		if ( (p = AST_LIST_NEXT(p, entry)) )
+	if (a->c->version == a->c_version && (p = a->obj)) {
+		if ((p = AST_LIST_NEXT(p, entry)))
 			goto found;
 		/* nope, start from the next bucket */
 		a->bucket++;
@@ -846,12 +873,24 @@ static void *internal_ao2_iterator_next(struct ao2_iterator *a, struct bucket_li
 
 found:
 	if (p) {
-		a->version = p->version;
-		a->obj = p;
-		a->c_version = a->c->version;
 		ret = EXTERNAL_OBJ(p->astobj);
-		/* inc refcount of returned object */
-		*q = p;
+		if (a->flags & AO2_ITERATOR_UNLINK) {
+			/* we are going to modify the container, so update version */
+			ast_atomic_fetchadd_int(&a->c->version, 1);
+			AST_LIST_REMOVE(&a->c->buckets[a->bucket], p, entry);
+			/* update number of elements */
+			ast_atomic_fetchadd_int(&a->c->elements, -1);
+			a->version = 0;
+			a->obj = NULL;
+			a->c_version = a->c->version;
+			ast_free(p);
+		} else {
+			a->version = p->version;
+			a->obj = p;
+			a->c_version = a->c->version;
+			/* inc refcount of returned object */
+			*q = p;
+		}
 	}
 
 	return ret;
diff --git a/main/channel.c b/main/channel.c
index 60ad9638b..af904692a 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1234,137 +1234,83 @@ struct ast_channel *ast_channel_callback(ao2_callback_data_fn *cb_fn, void *arg,
 }
 
 struct ast_channel_iterator {
-	struct ao2_iterator i;
-	const char *name;
-	size_t name_len;
-	const char *exten;
-	const char *context;
+	/* storage for non-dynamically allocated iterator */
+	struct ao2_iterator simple_iterator;
+	/* pointer to the actual iterator (simple_iterator or a dynamically
+	 * allocated iterator)
+	 */
+	struct ao2_iterator *active_iterator;
 };
 
 struct ast_channel_iterator *ast_channel_iterator_destroy(struct ast_channel_iterator *i)
 {
-	if (i->name) {
-		ast_free((void *) i->name);
-		i->name = NULL;
-	}
-
-	if (i->exten) {
-		ast_free((void *) i->exten);
-		i->exten = NULL;
-	}
-
-	if (i->context) {
-		ast_free((void *) i->context);
-		i->context = NULL;
-	}
-
+	ao2_iterator_destroy(i->active_iterator);
 	ast_free(i);
 
 	return NULL;
 }
 
-static struct ast_channel_iterator *ast_channel_iterator_new(int ao2_flags, const char *name,
-	size_t name_len, const char *exten, const char *context)
+static struct ast_channel_iterator *channel_iterator_search(const char *name,
+							    size_t name_len, const char *exten,
+							    const char *context)
 {
 	struct ast_channel_iterator *i;
+	struct ast_channel tmp_chan = {
+		.name = name,
+		/* This is sort of a hack.  Basically, we're using an arbitrary field
+		 * in ast_channel to pass the name_len for a prefix match.  If this
+		 * gets changed, then the compare callback must be changed, too. */
+		.rings = name_len,
+	};
 
 	if (!(i = ast_calloc(1, sizeof(*i)))) {
 		return NULL;
 	}
 
-	if (!ast_strlen_zero(exten) && !(i->exten = ast_strdup(exten))) {
-		goto return_error;
+	if (exten) {
+		ast_copy_string(tmp_chan.exten, exten, sizeof(tmp_chan.exten));
 	}
 
-	if (!ast_strlen_zero(context) && !(i->context = ast_strdup(context))) {
-		goto return_error;
+	if (context) {
+		ast_copy_string(tmp_chan.context, context, sizeof(tmp_chan.context));
 	}
 
-	if (!ast_strlen_zero(name) && !(i->name = ast_strdup(name))) {
-		goto return_error;
+	if (!(i->active_iterator = ao2_find(channels, &tmp_chan,
+					    OBJ_MULTIPLE | ((!ast_strlen_zero(name) && (name_len == 0)) ? OBJ_POINTER : 0)))) {
+		    ast_free(i);
+		    return NULL;
 	}
 
-	i->name_len = name_len;
-
-	i->i = ao2_iterator_init(channels, ao2_flags);
-
 	return i;
-
-return_error:
-	if (i->exten) {
-		ast_free((void *) i->exten);
-		i->exten = NULL;
-	}
-
-	if (i->context) {
-		ast_free((void *) i->context);
-		i->context = NULL;
-	}
-
-	ast_free(i);
-
-	return NULL;
-}
-
-struct ast_channel_iterator *ast_channel_iterator_by_exten_new(int ao2_flags, const char *exten,
-	const char *context)
-{
-	return ast_channel_iterator_new(ao2_flags, NULL, 0, exten, context);
 }
 
-struct ast_channel_iterator *ast_channel_iterator_by_name_new(int ao2_flags, const char *name,
-	size_t name_len)
+struct ast_channel_iterator *ast_channel_iterator_by_exten_new(const char *exten, const char *context)
 {
-	return ast_channel_iterator_new(ao2_flags, name, name_len, NULL, NULL);
+	return channel_iterator_search(NULL, 0, exten, context);
 }
 
-struct ast_channel_iterator *ast_channel_iterator_all_new(int ao2_flags)
+struct ast_channel_iterator *ast_channel_iterator_by_name_new(const char *name, size_t name_len)
 {
-	return ast_channel_iterator_new(ao2_flags, NULL, 0, NULL, NULL);
+	return channel_iterator_search(name, name_len, NULL, NULL);
 }
 
-/*!
- * \note This function will be reduced to 1 line of code once ao2 supports
- * returning multiple objects from an ao2_callback() using OBJ_MULTIPLE.
- */
-struct ast_channel *ast_channel_iterator_next(struct ast_channel_iterator *i)
+struct ast_channel_iterator *ast_channel_iterator_all_new(void)
 {
-	struct ast_channel *chan = NULL;
-
-	for (; (chan = ao2_iterator_next(&i->i));
-			ast_channel_unlock(chan), ast_channel_unref(chan)) {
-
-		ast_channel_lock(chan);
-
-		if (i->name) { /* match by name */
-			if (!i->name_len) {
-				if (strcasecmp(chan->name, i->name) && strcasecmp(chan->uniqueid, i->name)) {
-					continue; /* name match failed */
-				}
-			} else {
-				if (strncasecmp(chan->name, i->name, i->name_len) &&
-						strncasecmp(chan->uniqueid, i->name, i->name_len)) {
-					continue; /* name match failed */
-				}
-			}
-		} else if (i->exten) {
-			if (i->context && strcasecmp(chan->context, i->context) &&
-					strcasecmp(chan->macrocontext, i->context)) {
-				continue; /* context match failed */
-			}
+	struct ast_channel_iterator *i;
 
-			if (strcasecmp(chan->exten, i->exten) &&
-					strcasecmp(chan->macroexten, i->exten)) {
-				continue; /* exten match failed */
-			}
-		}
+	if (!(i = ast_calloc(1, sizeof(*i)))) {
+		return NULL;
+	}
 
-		ast_channel_unlock(chan);
+	i->simple_iterator = ao2_iterator_init(channels, 0);
+	i->active_iterator = &i->simple_iterator;
 
-		break; /* chan points to the next chan desired. */
-	}
+	return i;
+}
 
-	return chan;
+struct ast_channel *ast_channel_iterator_next(struct ast_channel_iterator *i)
+{
+	return ao2_iterator_next(i->active_iterator);
 }
 
 static int ast_channel_cmp_cb(void *obj, void *arg, int flags)
@@ -1407,7 +1353,7 @@ static int ast_channel_cmp_cb(void *obj, void *arg, int flags)
 }
 
 static struct ast_channel *ast_channel_get_full(const char *name, size_t name_len,
-	const char *exten, const char *context)
+						const char *exten, const char *context)
 {
 	struct ast_channel tmp_chan = {
 		.name = name,
diff --git a/main/cli.c b/main/cli.c
index 5b31bba2b..abfc8f0a9 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -827,7 +827,7 @@ static char *handle_chanlist(struct ast_cli_entry *e, int cmd, struct ast_cli_ar
 				"CallerID", "Duration", "Accountcode", "PeerAccount", "BridgedTo");
 	}
 
-	if (!count && !(iter = ast_channel_iterator_all_new(0))) {
+	if (!count && !(iter = ast_channel_iterator_all_new())) {
 		return CLI_FAILURE;
 	}
 
@@ -1454,7 +1454,7 @@ char *ast_complete_channels(const char *line, const char *word, int pos, int sta
 		return NULL;
 	}
 
-	if (!(iter = ast_channel_iterator_by_name_new(0, word, strlen(word)))) {
+	if (!(iter = ast_channel_iterator_by_name_new(word, strlen(word)))) {
 		return NULL;
 	}
 
@@ -1467,6 +1467,8 @@ char *ast_complete_channels(const char *line, const char *word, int pos, int sta
 		ast_channel_unref(c);
 	}
 
+	ast_channel_iterator_destroy(iter);
+
 	return ret == &notfound ? NULL : ret;
 }
 
diff --git a/main/manager.c b/main/manager.c
index 1bd2eaeb4..a02d06106 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -2806,7 +2806,7 @@ static int action_status(struct mansession *s, const struct message *m)
 	}
 
 	if (all) {
-		if (!(iter = ast_channel_iterator_all_new(0))) {
+		if (!(iter = ast_channel_iterator_all_new())) {
 			ast_free(str);
 			astman_send_error(s, m, "Memory Allocation Failure");
 			return 1;
@@ -2914,6 +2914,10 @@ static int action_status(struct mansession *s, const struct message *m)
 		}
 	}
 
+	if (iter) {
+		ast_channel_iterator_destroy(iter);
+	}
+
 	astman_append(s,
 		"Event: StatusComplete\r\n"
 		"%s"
@@ -3643,7 +3647,7 @@ static int action_coreshowchannels(struct mansession *s, const struct message *m
 		idText[0] = '\0';
 	}
 
-	if (!(iter = ast_channel_iterator_all_new(0))) {
+	if (!(iter = ast_channel_iterator_all_new())) {
 		astman_send_error(s, m, "Memory Allocation Failure");
 		return 1;
 	}