[Home]

Summary:ASTERISK-12422: [patch] Search channel by name prefix fails if a previous channel was defined in ast_channel_find_locked.
Reporter:Ramon Peek-Fares (ramonpeek)Labels:
Date Opened:2008-07-21 10:03:08Date Closed:2008-08-05 22:45:09
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/Channels
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) patch.diff
Description:Search channel by name prefix fails if a previous channel was defined in ast_channel_find_locked.

The function always return the previous value, because the previous channel value was constantly restored.
This caused failures in applications like Chanspy if searching for the next active channel with a certain prefix was used.
Comments:By: Ramon Peek-Fares (ramonpeek) 2008-07-21 10:04:24

The attached patch fixes this issue. ;-)

By: Mark Michelson (mmichelson) 2008-07-23 13:46:19

It doesn't make sense to me what the problem actually is in channel_find_locked. The current code looks right to me. I think that more likely than not, there are places where this function is being used incorrectly. In other words, it is the responsibility of the caller of channel_find_locked (or one of the other entry points into the function) to be sure that the prev input pointer is updated between calls.

You mentioned chanspy as an example where the function appeared broken. Could you outline how it's broken. I admit that in the case that a Zap/pseudo channel is found, it appears that the code does not properly update the "last" channel pointer as it should. I will get this fix merged.

By: Digium Subversion (svnbot) 2008-07-23 13:49:37

Repository: asterisk
Revision: 133101

U   branches/1.4/apps/app_chanspy.c

------------------------------------------------------------------------
r133101 | mmichelson | 2008-07-23 13:49:26 -0500 (Wed, 23 Jul 2008) | 6 lines

Update the "last" channel in next_channel in app_chanspy so
that the same pseudo channel isn't constantly returned.

related to issue ASTERISK-12422


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=133101

By: Ramon Peek-Fares (ramonpeek) 2008-07-24 02:29:11

The influence on Chanspy is as follows;

When spying on a specific peer that has multiple active calls, you should be able to walk through the active channels on that peer using the '*' key.
But without this fix ChanSpy would only spy on the first channel and never on any other channel since it could never find any other channel.

I'll elaborate a bit more, for example;
The first time chanspy uses the "ast_channel_find_locked" struct the previuos value is set to NULL, causing it to find the first channel active on the specified peer.
However after pressing the '*' key, "ast_channel_find_locked" is called a second time but now the 'prev' value is set.
This causes the "ast_channel_find_locked" to first lookup that channel, in order to continue the search from that channel on.
When it does it resets the 'prev' value to NULL like it should and breaks out of the loop.
But as soon as it breaks out of the loop, the 'prev' value is reset to the old value stored in '_prev' causing it to AGAIN start to search for this channel.
By moving the 'prev=_prev' outside this loop, this behaviour can be corrected

By: Digium Subversion (svnbot) 2008-08-05 22:45:07

Repository: asterisk
Revision: 135949

U   branches/1.4/main/channel.c

------------------------------------------------------------------------
r135949 | tilghman | 2008-08-05 22:45:06 -0500 (Tue, 05 Aug 2008) | 4 lines

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

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=135949