[asterisk-commits] rmudgett: trunk r373221 - in /trunk: ./ apps/ funcs/ include/asterisk/ main/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Sep 20 12:22:48 CDT 2012


Author: rmudgett
Date: Thu Sep 20 12:22:41 2012
New Revision: 373221

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=373221
Log:
Named call pickup groups. Fixes, missing functionality, and improvements.

* ASTERISK-20383
Missing named call pickup group features:

CHANNEL(callgroup) - Need CHANNEL(namedcallgroup)
CHANNEL(pickupgroup) - Need CHANNEL(namedpickupgroup)
Pickup() - Needs to also select from named pickup groups.

* ASTERISK-20384
Using the pickupexten, the pickup channel selection could fail even though
there was a call it could have picked up.  In a call pickup race when
there are multiple calls to pickup and two extensions try to pickup a
call, it is conceivable that the loser will not pick up any call even
though it could have picked up the next oldest matching call.

Regression because of the named call pickup group feature.

* See ASTERISK-20386 for the implementation improvements.  These are the
changes in channel.c and channel.h.

* Fixed some locking issues in CHANNEL().

(closes issue ASTERISK-20383)
Reported by: rmudgett
(closes issue ASTERISK-20384)
Reported by: rmudgett
(closes issue ASTERISK-20386)
Reported by: rmudgett
Tested by: rmudgett

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

Merged revisions 373220 from http://svn.asterisk.org/svn/asterisk/branches/11

Modified:
    trunk/   (props changed)
    trunk/apps/app_directed_pickup.c
    trunk/funcs/func_channel.c
    trunk/include/asterisk/channel.h
    trunk/include/asterisk/features.h
    trunk/main/channel.c
    trunk/main/features.c

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

Modified: trunk/apps/app_directed_pickup.c
URL: http://svnview.digium.com/svn/asterisk/trunk/apps/app_directed_pickup.c?view=diff&rev=373221&r1=373220&r2=373221
==============================================================================
--- trunk/apps/app_directed_pickup.c (original)
+++ trunk/apps/app_directed_pickup.c Thu Sep 20 12:22:41 2012
@@ -252,29 +252,13 @@
 	return res;
 }
 
-static int find_channel_by_group(void *obj, void *arg, void *data, int flags)
-{
-	struct ast_channel *target = obj;/*!< Potential pickup target */
-	struct ast_channel *chan = data;/*!< Channel wanting to pickup call */
-
-	ast_channel_lock(target);
-	if (chan != target && (ast_channel_pickupgroup(chan) & ast_channel_callgroup(target))
-		&& ast_can_pickup(target)) {
-		/* Return with the channel still locked on purpose */
-		return CMP_MATCH | CMP_STOP;
-	}
-	ast_channel_unlock(target);
-
-	return 0;
-}
-
 static int pickup_by_group(struct ast_channel *chan)
 {
 	struct ast_channel *target;/*!< Potential pickup target */
 	int res = -1;
 
 	/* The found channel is already locked. */
-	target = ast_channel_callback(find_channel_by_group, NULL, chan, 0);
+	target = ast_pickup_find_by_group(chan);
 	if (target) {
 		ast_log(LOG_NOTICE, "pickup %s attempt by %s\n", ast_channel_name(target), ast_channel_name(chan));
 		res = ast_do_pickup(chan, target);

Modified: trunk/funcs/func_channel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/funcs/func_channel.c?view=diff&rev=373221&r1=373220&r2=373221
==============================================================================
--- trunk/funcs/func_channel.c (original)
+++ trunk/funcs/func_channel.c Thu Sep 20 12:22:41 2012
@@ -87,10 +87,16 @@
 						<para>R/O format currently being written.</para>
 					</enum>
 					<enum name="callgroup">
-						<para>R/W call groups for call pickup.</para>
+						<para>R/W numeric call pickup groups that this channel is a member.</para>
 					</enum>
 					<enum name="pickupgroup">
-						<para>R/W call groups for call pickup.</para>
+						<para>R/W numeric call pickup groups this channel can pickup.</para>
+					</enum>
+					<enum name="namedcallgroup">
+						<para>R/W named call pickup groups that this channel is a member.</para>
+					</enum>
+					<enum name="namedpickupgroup">
+						<para>R/W named call pickup groups this channel can pickup.</para>
 					</enum>
 					<enum name="channeltype">
 						<para>R/O technology used for channel.</para>
@@ -346,15 +352,18 @@
 			     char *data, char *buf, size_t len)
 {
 	int ret = 0;
-	char tmp[512];
 	struct ast_format_cap *tmpcap;
 
 	if (!strcasecmp(data, "audionativeformat")) {
+		char tmp[512];
+
 		if ((tmpcap = ast_format_cap_get_type(ast_channel_nativeformats(chan), AST_FORMAT_TYPE_AUDIO))) {
 			ast_copy_string(buf, ast_getformatname_multiple(tmp, sizeof(tmp), tmpcap), len);
 			tmpcap = ast_format_cap_destroy(tmpcap);
 		}
 	} else if (!strcasecmp(data, "videonativeformat")) {
+		char tmp[512];
+
 		if ((tmpcap = ast_format_cap_get_type(ast_channel_nativeformats(chan), AST_FORMAT_TYPE_VIDEO))) {
 			ast_copy_string(buf, ast_getformatname_multiple(tmp, sizeof(tmp), tmpcap), len);
 			tmpcap = ast_format_cap_destroy(tmpcap);
@@ -417,6 +426,7 @@
 		ast_channel_unlock(chan);
 	} else if (!strcasecmp(data, "peer")) {
 		struct ast_channel *p;
+
 		ast_channel_lock(chan);
 		p = ast_bridged_channel(chan);
 		if (p || ast_channel_tech(chan) || ast_channel_cdr(chan)) /* dummy channel? if so, we hid the peer name in the language */
@@ -437,16 +447,27 @@
 		locked_copy_string(chan, buf, transfercapability_table[ast_channel_transfercapability(chan) & 0x1f], len);
 	} else if (!strcasecmp(data, "callgroup")) {
 		char groupbuf[256];
+
 		locked_copy_string(chan, buf,  ast_print_group(groupbuf, sizeof(groupbuf), ast_channel_callgroup(chan)), len);
 	} else if (!strcasecmp(data, "pickupgroup")) {
 		char groupbuf[256];
+
 		locked_copy_string(chan, buf,  ast_print_group(groupbuf, sizeof(groupbuf), ast_channel_pickupgroup(chan)), len);
+	} else if (!strcasecmp(data, "namedcallgroup")) {
+		struct ast_str *tmp_str = ast_str_alloca(1024);
+
+		locked_copy_string(chan, buf,  ast_print_namedgroups(&tmp_str, ast_channel_named_callgroups(chan)), len);
+	} else if (!strcasecmp(data, "namedpickupgroup")) {
+		struct ast_str *tmp_str = ast_str_alloca(1024);
+
+		locked_copy_string(chan, buf,  ast_print_namedgroups(&tmp_str, ast_channel_named_pickupgroups(chan)), len);
 	} else if (!strcasecmp(data, "amaflags")) {
-		char amabuf[256];
-		snprintf(amabuf,sizeof(amabuf), "%d", ast_channel_amaflags(chan));
-		locked_copy_string(chan, buf, amabuf, len);
+		ast_channel_lock(chan);
+		snprintf(buf, len, "%d", ast_channel_amaflags(chan));
+		ast_channel_unlock(chan);
 	} else if (!strncasecmp(data, "secure_bridge_", 14)) {
 		struct ast_datastore *ds;
+
 		ast_channel_lock(chan);
 		if ((ds = ast_channel_datastore_find(chan, &secure_call_info, NULL))) {
 			struct ast_secure_call_store *encrypt = ds->data;
@@ -529,9 +550,27 @@
 			new_zone = ast_tone_zone_unref(new_zone);
 		}
 	} else if (!strcasecmp(data, "callgroup")) {
+		ast_channel_lock(chan);
 		ast_channel_callgroup_set(chan, ast_get_group(value));
+		ast_channel_unlock(chan);
 	} else if (!strcasecmp(data, "pickupgroup")) {
+		ast_channel_lock(chan);
 		ast_channel_pickupgroup_set(chan, ast_get_group(value));
+		ast_channel_unlock(chan);
+	} else if (!strcasecmp(data, "namedcallgroup")) {
+		struct ast_namedgroups *groups = ast_get_namedgroups(value);
+
+		ast_channel_lock(chan);
+		ast_channel_named_callgroups_set(chan, groups);
+		ast_channel_unlock(chan);
+		ast_unref_namedgroups(groups);
+	} else if (!strcasecmp(data, "namedpickupgroup")) {
+		struct ast_namedgroups *groups = ast_get_namedgroups(value);
+
+		ast_channel_lock(chan);
+		ast_channel_named_pickupgroups_set(chan, groups);
+		ast_channel_unlock(chan);
+		ast_unref_namedgroups(groups);
 	} else if (!strcasecmp(data, "txgain")) {
 		sscanf(value, "%4hhd", &gainset);
 		ast_channel_setoption(chan, AST_OPTION_TXGAIN, &gainset, sizeof(gainset), 0);
@@ -540,12 +579,15 @@
 		ast_channel_setoption(chan, AST_OPTION_RXGAIN, &gainset, sizeof(gainset), 0);
 	} else if (!strcasecmp(data, "transfercapability")) {
 		unsigned short i;
+
+		ast_channel_lock(chan);
 		for (i = 0; i < 0x20; i++) {
 			if (!strcasecmp(transfercapability_table[i], value) && strcmp(value, "UNK")) {
 				ast_channel_transfercapability_set(chan, i);
 				break;
 			}
 		}
+		ast_channel_unlock(chan);
 	} else if (!strcasecmp(data, "hangup_handler_pop")) {
 		/* Pop one hangup handler before pushing the new handler. */
 		ast_pbx_hangup_handler_pop(chan);
@@ -581,13 +623,13 @@
 		} else {
 			store = ds->data;
 		}
-		ast_channel_unlock(chan);
 
 		if (!strcasecmp(data, "secure_bridge_signaling")) {
 			store->signaling = ast_true(value) ? 1 : 0;
 		} else if (!strcasecmp(data, "secure_bridge_media")) {
 			store->media = ast_true(value) ? 1 : 0;
 		}
+		ast_channel_unlock(chan);
 	} else if (!ast_channel_tech(chan)->func_channel_write
 		 || ast_channel_tech(chan)->func_channel_write(chan, function, data, value)) {
 		ast_log(LOG_WARNING, "Unknown or unavailable item requested: '%s'\n",

Modified: trunk/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/channel.h?view=diff&rev=373221&r1=373220&r2=373221
==============================================================================
--- trunk/include/asterisk/channel.h (original)
+++ trunk/include/asterisk/channel.h Thu Sep 20 12:22:41 2012
@@ -999,17 +999,6 @@
 	CHANNEL_ACL_RELOAD,
 };
 
-
-/*! \brief Structure to handle ao2-container for named groups */
-struct namedgroup_entry {
-	/*! string representation of group */
-	char *name;
-
-	/*! pre-built hash of groupname string */
-	unsigned int hash;
-};
-
-
 /*!
  * \note None of the datastore API calls lock the ast_channel they are using.
  *       So, the channel should be locked before calling the functions that
@@ -2454,20 +2443,20 @@
 
 ast_group_t ast_get_group(const char *s);
 
-/*! \brief Print call- and pickup groups into buffer */
+/*! \brief Print call and pickup groups into buffer */
 char *ast_print_group(char *buf, int buflen, ast_group_t group);
 
 /*! \brief Opaque struct holding a namedgroups set, i.e. a set of group names */
 struct ast_namedgroups;
 
-/*! \brief Create an ast_namedgroups set with group name from comma separated string s */
+/*! \brief Create an ast_namedgroups set with group names from comma separated string */
 struct ast_namedgroups *ast_get_namedgroups(const char *s);
 struct ast_namedgroups *ast_unref_namedgroups(struct ast_namedgroups *groups);
 struct ast_namedgroups *ast_ref_namedgroups(struct ast_namedgroups *groups);
 /*! \brief Return TRUE if group a and b contain at least one common groupname */
 int ast_namedgroups_intersect(struct ast_namedgroups *a, struct ast_namedgroups *b);
 
-/*! \brief Print named call groups and named pickup groups ---*/
+/*! \brief Print named call groups and named pickup groups */
 char *ast_print_namedgroups(struct ast_str **buf, struct ast_namedgroups *groups);
 
 /*!

Modified: trunk/include/asterisk/features.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/features.h?view=diff&rev=373221&r1=373220&r2=373221
==============================================================================
--- trunk/include/asterisk/features.h (original)
+++ trunk/include/asterisk/features.h Thu Sep 20 12:22:41 2012
@@ -183,6 +183,16 @@
  */
 int ast_can_pickup(struct ast_channel *chan);
 
+/*!
+ * \brief Find a pickup channel target by group.
+ *
+ * \param chan channel that initiated pickup.
+ *
+ * \retval target on success.  The returned channel is locked and reffed.
+ * \retval NULL on error.
+ */
+struct ast_channel *ast_pickup_find_by_group(struct ast_channel *chan);
+
 /*! \brief Pickup a call */
 int ast_pickup_call(struct ast_channel *chan);
 

Modified: trunk/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/channel.c?view=diff&rev=373221&r1=373220&r2=373221
==============================================================================
--- trunk/main/channel.c (original)
+++ trunk/main/channel.c Thu Sep 20 12:22:41 2012
@@ -8220,97 +8220,86 @@
 	return group;
 }
 
+/*! \brief Named group member structure */
+struct namedgroup_member {
+	/*! Pre-built hash of group member name. */
+	unsigned int hash;
+	/*! Group member name. (End allocation of name string.) */
+	char name[1];
+};
+
 /*! \brief Comparison function used for named group container */
 static int namedgroup_cmp_cb(void *obj, void *arg, int flags)
 {
-	const struct namedgroup_entry *an = obj;
-	const struct namedgroup_entry *bn = arg;
+	const struct namedgroup_member *an = obj;
+	const struct namedgroup_member *bn = arg;
+
 	return strcmp(an->name, bn->name) ? 0 : CMP_MATCH | CMP_STOP;
 }
 
 /*! \brief Hashing function used for named group container */
 static int namedgroup_hash_cb(const void *obj, const int flags)
 {
-	const struct namedgroup_entry *entry = obj;
-	return entry->hash;
-}
-
-static void namedgroup_dtor(void *obj)
-{
-	ast_free(((struct namedgroup_entry*)obj)->name);
-}
-
-/*! \internal \brief Actually a refcounted ao2_object. An indirect ao2_container to hide the implementation of namedgroups. */
-struct ast_namedgroups {
-	struct ao2_container *container;
-};
-
-static void ast_namedgroups_dtor(void *obj)
-{
-	ao2_cleanup(((struct ast_namedgroups *)obj)->container);
+	const struct namedgroup_member *member = obj;
+
+	return member->hash;
 }
 
 struct ast_namedgroups *ast_get_namedgroups(const char *s)
 {
-	struct ast_namedgroups *namedgroups;
+	struct ao2_container *namedgroups;
 	char *piece;
 	char *c;
 
-	if (ast_strlen_zero(s)) {
+	if (!s) {
 		return NULL;
 	}
-	c = ast_strdupa(s);
-	if (!c) {
+
+	/*! \brief Remove leading and trailing whitespace */
+	c = ast_trim_blanks(ast_strdupa(ast_skip_blanks(s)));
+	if (ast_strlen_zero(c)) {
 		return NULL;
 	}
 
-	namedgroups = ao2_alloc_options(sizeof(*namedgroups), ast_namedgroups_dtor, AO2_ALLOC_OPT_LOCK_NOLOCK);
+	namedgroups = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, 19,
+		namedgroup_hash_cb, namedgroup_cmp_cb);
 	if (!namedgroups) {
 		return NULL;
 	}
-	namedgroups->container = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, 19, namedgroup_hash_cb, namedgroup_cmp_cb);
-	if (!namedgroups->container) {
-		ao2_ref(namedgroups, -1);
-		return NULL;
-	}
-
-	/*! \brief Remove leading and trailing whitespace */
-	c = ast_strip(c);
 
 	while ((piece = strsep(&c, ","))) {
-		struct namedgroup_entry *entry;
+		struct namedgroup_member *member;
+		size_t len;
 
 		/* remove leading/trailing whitespace */
 		piece = ast_strip(piece);
-		if (strlen(piece) == 0) {
-			ast_log(LOG_ERROR, "Syntax error parsing named group configuration '%s' at '%s'. Ignoring.\n", s, piece);
+
+		len = strlen(piece);
+		if (!len) {
 			continue;
 		}
 
-		entry = ao2_alloc_options(sizeof(*entry), namedgroup_dtor, AO2_ALLOC_OPT_LOCK_NOLOCK);
-		if (!entry) {
+		member = ao2_alloc_options(sizeof(*member) + len, NULL, AO2_ALLOC_OPT_LOCK_NOLOCK);
+		if (!member) {
 			ao2_ref(namedgroups, -1);
 			return NULL;
 		}
-		entry->name = ast_strdup(piece);
-		if (!entry->name) {
-			ao2_ref(entry, -1);
-			ao2_ref(namedgroups, -1);
-			return NULL;
-		}
-		entry->hash = ast_str_hash(entry->name);
+		strcpy(member->name, piece);/* Safe */
+		member->hash = ast_str_hash(member->name);
+
 		/* every group name may exist only once, delete duplicates */
-		ao2_find(namedgroups->container, entry, OBJ_POINTER | OBJ_UNLINK | OBJ_NODATA);
-		ao2_link(namedgroups->container, entry);
-		ao2_ref(entry, -1);
-	}
-
-	if (ao2_container_count(namedgroups->container) == 0) {
+		ao2_find(namedgroups, member, OBJ_POINTER | OBJ_UNLINK | OBJ_NODATA);
+		ao2_link(namedgroups, member);
+		ao2_ref(member, -1);
+	}
+
+	if (!ao2_container_count(namedgroups)) {
+		/* There were no group names specified. */
 		ao2_ref(namedgroups, -1);
 		namedgroups = NULL;
 	}
 
-	return namedgroups;
+	return (struct ast_namedgroups *) namedgroups;
 }
 
 struct ast_namedgroups *ast_unref_namedgroups(struct ast_namedgroups *groups)
@@ -8560,26 +8549,24 @@
 	return buf;
 }
 
-/*! \brief Print named call group and named pickup group ---*/
 char *ast_print_namedgroups(struct ast_str **buf, struct ast_namedgroups *group)
 {
-	struct namedgroup_entry *ng;
+	struct ao2_container *grp = (struct ao2_container *) group;
+	struct namedgroup_member *ng;
 	int first = 1;
 	struct ao2_iterator it;
 
-	if (group == NULL) {
+	if (!grp) {
 		return ast_str_buffer(*buf);
 	}
 
-	it = ao2_iterator_init(group->container, 0);
-	while ((ng = ao2_iterator_next(&it))) {
+	for (it = ao2_iterator_init(grp, 0); (ng = ao2_iterator_next(&it)); ao2_ref(ng, -1)) {
 		if (!first) {
 			ast_str_append(buf, 0, ", ");
 		} else {
 			first = 0;
 		}
 		ast_str_append(buf, 0, "%s", ng->name);
-		ao2_ref(ng, -1);
 	}
 	ao2_iterator_destroy(&it);
 
@@ -8598,16 +8585,24 @@
 
 int ast_namedgroups_intersect(struct ast_namedgroups *a, struct ast_namedgroups *b)
 {
-	/*
-	 * Do a and b intersect? Since b is hash table average time complexity is O(|a|)
-	 */
 	void *match;
+	struct ao2_container *group_a = (struct ao2_container *) a;
+	struct ao2_container *group_b = (struct ao2_container *) b;
 
 	if (!a || !b) {
 		return 0;
 	}
 
-	match = ao2_callback(a->container, 0, namedgroup_match, b->container);
+	/*
+	 * Do groups a and b intersect?  Since a and b are hash tables,
+	 * the average time complexity is:
+	 * O(a.count <= b.count ? a.count : b.count)
+	 */
+	if (ao2_container_count(group_b) < ao2_container_count(group_a)) {
+		/* Traverse over the smaller group. */
+		SWAP(group_a, group_b);
+	}
+	match = ao2_callback(group_a, 0, namedgroup_match, group_b);
 	ao2_cleanup(match);
 
 	return match != NULL;

Modified: trunk/main/features.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/features.c?view=diff&rev=373221&r1=373220&r2=373221
==============================================================================
--- trunk/main/features.c (original)
+++ trunk/main/features.c Thu Sep 20 12:22:41 2012
@@ -7707,26 +7707,98 @@
 static int find_channel_by_group(void *obj, void *arg, void *data, int flags)
 {
 	struct ast_channel *target = obj;/*!< Potential pickup target */
-	struct ast_channel *chan = data;/*!< Channel wanting to pickup call */
+	struct ast_channel *chan = arg;/*!< Channel wanting to pickup call */
+
+	if (chan == target) {
+		return 0;
+	}
 
 	ast_channel_lock(target);
-
-	/*
-	 * Both, callgroup and namedcallgroup pickup variants are matched independently.
-	 * Checking for named group match is done last since it's a rather expensive operation.
-	 */
-	if (chan != target && ast_can_pickup(target)
-		&& ((ast_channel_pickupgroup(chan) & ast_channel_callgroup(target))
-			|| (ast_namedgroups_intersect(ast_channel_named_pickupgroups(chan), ast_channel_named_callgroups(target))))) {
+	if (ast_can_pickup(target)) {
+		/* Lock both channels. */
+		while (ast_channel_trylock(chan)) {
+			ast_channel_unlock(target);
+			sched_yield();
+			ast_channel_lock(target);
+		}
+
 		/*
-		 * Return with the channel unlocked on purpose, else we would lock many channels with the chance for deadlock
+		 * Both callgroup and namedcallgroup pickup variants are
+		 * matched independently.  Checking for named group match is
+		 * done last since it's a more expensive operation.
 		 */
+		if ((ast_channel_pickupgroup(chan) & ast_channel_callgroup(target))
+			|| (ast_namedgroups_intersect(ast_channel_named_pickupgroups(chan),
+				ast_channel_named_callgroups(target)))) {
+			struct ao2_container *candidates = data;/*!< Candidate channels found. */
+
+			/* This is a candidate to pickup */
+			ao2_link(candidates, target);
+		}
+		ast_channel_unlock(chan);
+	}
+	ast_channel_unlock(target);
+
+	return 0;
+}
+
+struct ast_channel *ast_pickup_find_by_group(struct ast_channel *chan)
+{
+	struct ao2_container *candidates;/*!< Candidate channels found to pickup. */
+	struct ast_channel *target;/*!< Potential pickup target */
+
+	candidates = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, 1, NULL, NULL);
+	if (!candidates) {
+		return NULL;
+	}
+
+	/* Find all candidate targets by group. */
+	ast_channel_callback(find_channel_by_group, chan, candidates, 0);
+
+	/* Find the oldest pickup target candidate */
+	target = NULL;
+	for (;;) {
+		struct ast_channel *candidate;/*!< Potential new older target */
+		struct ao2_iterator iter;
+
+		iter = ao2_iterator_init(candidates, 0);
+		while ((candidate = ao2_iterator_next(&iter))) {
+			if (!target) {
+				/* First target. */
+				target = candidate;
+				continue;
+			}
+			if (ast_tvcmp(ast_channel_creationtime(candidate), ast_channel_creationtime(target)) < 0) {
+				/* We have a new target. */
+				ast_channel_unref(target);
+				target = candidate;
+				continue;
+			}
+			ast_channel_unref(candidate);
+		}
+		ao2_iterator_destroy(&iter);
+		if (!target) {
+			/* No candidates found. */
+			break;
+		}
+
+		/* The found channel must be locked and ref'd. */
+		ast_channel_lock(target);
+
+		/* Recheck pickup ability */
+		if (ast_can_pickup(target)) {
+			/* This is the channel to pickup. */
+			break;
+		}
+
+		/* Someone else picked it up or the call went away. */
 		ast_channel_unlock(target);
-		return CMP_MATCH;
-	}
-	ast_channel_unlock(target);
-
-	return 0;
+		ao2_unlink(candidates, target);
+		target = ast_channel_unref(target);
+	}
+	ao2_ref(candidates, -1);
+
+	return target;
 }
 
 /*!
@@ -7740,47 +7812,12 @@
 int ast_pickup_call(struct ast_channel *chan)
 {
 	struct ast_channel *target;/*!< Potential pickup target */
-	struct ao2_iterator *targets_it;/*!< Potential pickup targets, must find the oldest of them */
-	struct ast_channel *candidate;/*!< Potential new older target */
 	int res = -1;
+
 	ast_debug(1, "pickup attempt by %s\n", ast_channel_name(chan));
 
-	/*
-	 * Transfer all pickup-able channels to another container-iterator.
-	 * Iterate it to find the oldest channel.
-	 */
-	targets_it = (struct ao2_iterator *) ast_channel_callback(find_channel_by_group,
-		NULL, chan, OBJ_MULTIPLE);
-	if (!targets_it) {
-		/* Search really failed. */
-		goto no_pickup_calls;
-	}
-
-	target = NULL;
-	while ((candidate = ao2_iterator_next(targets_it))) {
-		if (!target) {
-			target = candidate;
-			continue;
-		}
-		if (ast_tvcmp(ast_channel_creationtime(candidate), ast_channel_creationtime(target)) < 0) {
-			ast_channel_unref(target);
-			target = candidate;
-			continue;
-		}
-		ast_channel_unref(candidate);
-	}
-	ao2_iterator_destroy(targets_it);
-	if (target) {
-		/* The found channel must be locked and ref'd. */
-		ast_channel_lock(target);
-		/* Recheck pickup ability */
-		if (!ast_can_pickup(target)) {
-			/* Someone else picked it up or the call went away. */
-			ast_channel_unlock(target);
-			target = ast_channel_unref(target);
-		}
-	}
-
+	/* The found channel is already locked. */
+	target = ast_pickup_find_by_group(chan);
 	if (target) {
 		ast_log(LOG_NOTICE, "pickup %s attempt by %s\n", ast_channel_name(target), ast_channel_name(chan));
 
@@ -7796,7 +7833,6 @@
 		target = ast_channel_unref(target);
 	}
 
-no_pickup_calls:
 	if (res < 0) {
 		ast_debug(1, "No call pickup possible... for %s\n", ast_channel_name(chan));
 		if (!ast_strlen_zero(pickupfailsound)) {




More information about the asterisk-commits mailing list