[Home]

Summary:ASTERISK-17797: Coding error in find_subchannel_and_lock()
Reporter:Jeff Waltz (jeffw)Labels:
Date Opened:2011-05-04 12:39:14Date Closed:2014-11-08 18:44:10.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_mgcp
Versions:1.8.3 Frequency of
Occurrence
Related
Issues:
is duplicated byASTERISK-24500 Regression introduced in chan_mgcp by SVN revision r227276
Environment:Attachments:
Description:The function find_subchannel_and_lock()in mgcp_chan.c has a coding error in two locations that prevent the subchannel from being found on non-dynamic gateways. This results in responses from half the gateways to be ignored.  I built Asterisk from source on a Fedora 14 system.  Here is the C code in question from Asterisk version 1.8.3.2:

{code}
for (g = gateways ? gateways : find_realtime_gw(name, at, sin);
               g; g = g->next ? g->next : find_realtime_gw(name, at, sin)) {

       [lines omitted]

                       /* not dynamic, check if the name matches */
                       } else if (name) {
                            if (strcasecmp(g->name, at)) {
>>>> error >>>>                 g = g->next;
                               continue;
                            }
                       /* not dynamic, no name, check if the addr matches */
                       } else if (!name && sin) {
                            if ((g->addr.sin_addr.s_addr !=
                                 sin->sin_addr.s_addr) ||
                                (g->addr.sin_port != sin->sin_port)) {
>>>> error >>>>                  if(!g->next)
>>>> error >>>>                        g = find_realtime_gw(name, at, sin);
>>>> error >>>>                   else
>>>> error >>>>                        g = g->next;
                                 continue;
                           }
                       } else {
                           continue;
                       }
{code}

Since the "for" statement includes updating "g" from "g->next", the extra updates to "g" in the code above cause gateway entries to be skipped and responses from those gateways to continually exceed the maximum retry count.

I hope this helps!

Jeff Waltz

****** ADDITIONAL INFORMATION ******

I just downloaded the source for 1.8.4 and the erroneous code is still present.
Comments:By: Jeff Waltz (jeffw) 2011-05-04 13:23:44

I loaded 1.8.3.3 not 1.8.4, to double check that the error is still present.
By: Jeff Waltz (jeffw) 2011-06-02 15:22:03

Here is a diff of the changes I made to correct this problem:

1802d1801
< g = g->next;
1809,1812d1807
< if(!g->next)
< g = find_realtime_gw(name, at, sin);
< else
< g = g->next;
By: Nenad Kljajic (foundanswer) 2011-06-09 04:22:32.377-0500

For loop does not check for null pointer.


1776c1778
<       for (g = gateways ? gateways : find_realtime_gw(name, at, sin); g; g = g->next ? g->next : find_realtime_gw(name, at, sin)) {
---
>       for (g = gateways ? gateways : find_realtime_gw(name, at, sin); g; g = g ? (g->next ? g->next : find_realtime_gw(name, at, sin)) : NULL ) {

By: Matt Jordan (mjordan) 2014-11-08 18:44:10.122-0600

This was fixed by a patch provided by Xavier Hienne in ASTERISK-24500.