[asterisk-commits] tilghman: branch 1.6.0 r135951 - in /branches/1.6.0: ./ main/channel.c

[SVN commits to the Asterisk project]

Author: tilghman
Date: Tue Aug  5 22:56:30 2008
New Revision: 135951

URL: http://svn.digium.com/view/asterisk?view=rev&rev=135951
Log:
Merged revisions 135950 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/trunk

................
r135950 | tilghman | 2008-08-05 22:55:49 -0500 (Tue, 05 Aug 2008) | 12 lines

Merged revisions 135949 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r135949 | tilghman | 2008-08-05 22:53:36 -0500 (Tue, 05 Aug 2008) | 4 lines

Fix a longstanding bug in channel walking logic, and fix the explanation to
make sense.
(Closes issue #13124)

........

................

Modified:
    branches/1.6.0/   (props changed)
    branches/1.6.0/main/channel.c

Propchange: branches/1.6.0/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.

Modified: branches/1.6.0/main/channel.c
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/main/channel.c?view=diff&rev=135951&r1=135950&r2=135951
==============================================================================
--- branches/1.6.0/main/channel.c (original)
+++ branches/1.6.0/main/channel.c Tue Aug  5 22:56:30 2008
@@ -1102,24 +1102,27 @@
 
 	for (retries = 0; retries < 200; retries++) {
 		int done;
+		/* Reset prev on each retry.  See note below for the reason. */
+		prev = _prev;
 		AST_RWLIST_RDLOCK(&channels);
 		AST_RWLIST_TRAVERSE(&channels, c, chan_list) {
-			prev = _prev;
-			if (prev) {	/* look for next item */
+			if (prev) {	/* look for last item, first, before any evaluation */
 				if (c != prev)	/* not this one */
 					continue;
 				/* found, prepare to return c->next */
 				if ((c = AST_RWLIST_NEXT(c, chan_list)) == NULL) break;
-				/* If prev was the last item on the channel list, then we just
-				 * want to return NULL, instead of trying to deref NULL in the
-				 * next section.
+				/*!\note
+				 * We're done searching through the list for the previous item.
+				 * Any item after this point, we want to evaluate for a match.
+				 * If we didn't set prev to NULL here, then we would only
+				 * return matches for the first matching item (since the above
+				 * "if (c != prev)" would not permit any other potential
+				 * matches to reach the additional matching logic, below).
+				 * Instead, it would just iterate until it once again found the
+				 * original match, then iterate down to the end of the list and
+				 * quit.
 				 */
 				prev = NULL;
-				/* We want prev to be NULL in case we end up doing more searching through
-				 * the channel list to find the channel (ie: name searching). If we didn't
-				 * set this to NULL the logic would just blow up
-				 * XXX Need a better explanation for this ...
-				 */
 			}
 			if (name) { /* want match by name */
 				if ((!namelen && strcasecmp(c->name, name) && strcmp(c->uniqueid, name)) ||