Fix a longstanding bug in channel walking logic, and fix the explanation to

make sense.
(Closes issue #13124)

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

diff --git a/main/channel.c b/main/channel.c
index ecf84ac213ee8cd945d10dd554b62c1b0b464bc1..3006e71cb24569273cbe47dfa133de048dbd49c4 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -1043,24 +1043,27 @@ static struct ast_channel *channel_find_locked(const struct ast_channel *prev,
 
        for (retries = 0; retries < 200; retries++) {
                int done;
+               /* Reset prev on each retry.  See note below for the reason. */
+               prev = _prev;
                AST_LIST_LOCK(&channels);
                AST_LIST_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_LIST_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)) ||