[asterisk-dev] Strange code in chan_zap.c ?

[Tzafrir Cohen]

On Tue, Oct 31, 2006 at 04:48:04PM +0000, Tony Mountifield wrote:
> While working through chan_zap.c for a particular issue, I came across
> the following code, which, although unrelated to what I was looking for,
> didn't look right (this is one case within a switch statement):
> 
> 	/* FGD MF *Must* wait for wink */
> 	if (!ast_strlen_zero(p->dop.dialstr))
> 		res = ioctl(p->subs[SUB_REAL].zfd, ZT_DIAL, &p->dop);
> 	else if (res < 0) {
> 		ast_log(LOG_WARNING, "Unable to initiate dialing on trunk channel %d\n", p->channel);
> 		p->dop.dialstr[0] = '\0';
> 		return NULL;
> 	} else
> 		ast_log(LOG_DEBUG, "Sent deferred digit string: %s\n", p->dop.dialstr);
> 	p->dop.dialstr[0] = '\0';
> 	break;
> 
> It looks like the "if (res < 0)" is supposed to be checking the return value
> from the ioctl, but in fact is hidden by the "else".
> 
> Unless I'm misunderstanding the code, I think it should be something like:
> 
> 	/* FGD MF *Must* wait for wink */
> 	if (!ast_strlen_zero(p->dop.dialstr)) {
> 		res = ioctl(p->subs[SUB_REAL].zfd, ZT_DIAL, &p->dop);
> 		if (res < 0) {
> 			ast_log(LOG_WARNING, "Unable to initiate dialing on trunk channel %d\n", p->channel);
> 			p->dop.dialstr[0] = '\0';
> 			return NULL;
> 		} else
> 			ast_log(LOG_DEBUG, "Sent deferred digit string: %s\n", p->dop.dialstr);
> 		p->dop.dialstr[0] = '\0';
> 	}
> 	break;
> 
> It is the same in 1.2 and trunk.

One similar thing:

trunk/channels/chan_zap.c

zt_get_event() is used in zt_handle_event(), __zt_exception(),
ss_thread(), do_monitor() and perhaps several other places. 
zt_get_event may return -1 which means that the ioctl ZT_GETEVENT has
failed. Yet none of the callers seems to check for res < 0. 

One problematic case: 'zap restart' on a system that has a ISDN span.
zap restart destroys all the (B)channels. The D channel is left open.
Somehow then setup_zap(0) then gets zaptel into a loop where it
constantly tries to read events on the D channel. An ioctl fails, and it
keeps "getting an unknown event".

Don't try this with asterisk -p unless you have a manager port open, as
it sets Asterisk into a 100% CPU loop.

-- 
               Tzafrir Cohen       
icq#16849755                    jabber:tzafrir at jabber.org
+972-50-7952406           mailto:tzafrir.cohen at xorcom.com       
http://www.xorcom.com  iax:guest at local.xorcom.com/tzafrir