[asterisk-dev] bug 7506 -- This may be a life-changing experience to channel programmers! Please read!

[Steve Murphy]

Bug 7506 complains that Newchannel and Newstate  are getting generated
at the wrong times. 

And so it is! 

The submitter generated a patch, but this was just a quick-fix. It isn't
the "right thing". 

Russell suggested the ideal spot to generate a Newchannel event is in
the channel_alloc func (or is it ast_channel_alloc?) (Whatever it's
called in real life, I refer to it simply as "channel_alloc" from here
on for brevity. Deal with that, please.) 

At any rate, he's right. This is the "right thing" to do. It's the most
logical spot to put it. 

So I did it. But it got complicated...

To generate the current Newchannel event, you need the name, uniqueid,
callerid{name,num}, and state info. 

So, no surprise, I added these args to the channel_alloc func: 
int state, char* cid_num, char *cid_name. 

Just about every channel uses the stringfield build operator to
basically sprintf the name of the channel.  So, I also added (char
*name_fmt, ...) to the channel_alloc() arglist, at the end. Logical,
right? 

These are the complications: 

1. All the funcs, almost, use the ast_setstate() func to actually set
the state, and generate the newchannel event. 
   Well, you could also use it to generate the Newstate event. Well,
now, ast_setstate sets the state and generates just the Newstate event,
and nothing more or less. So every time it was used to to generate the
Newchannel event, I erased it. the channel_alloc func sets the state.
That state appears in the Newchannel event. No need to immediately
generate a Newstate event with the same info that was in the preceding
Newchannel event. 

2. The Newchannel event contains the cid info. No need (in most cases)
to call set_callerid to set the cid struct in the channel, and generate
an immediately following Newcallerid event. So, I scoured the channel
source files and swapped set_callerid calls for plain sets of the cid
fields. A few channels were generating erroneous newcallerid events, I
think chan_gtalk, and chan_oss. So I removed them, and replaced them
with code to set the values without generating an event. Why didn't I
just have channel_alloc() set the callerid's in the channel struct
instead of just including that info in the event? Because NOT ALL the
channels do this, one or two oddballs want to use the set_callerid func
to do it and generate the event, and I think, rightly so. So, I let the
channel code do that, as it has up to now. 

3. Two wise-guy channels, chan_features and chan_zap, did a little
tap-dance with the name stringfield to generate a name that was unique.
Iterating thru like 3 different possible names based on an index. I
invented the func ast_safe_string_alloc() to build the proposed names
instead, passing the winner into the channel_alloc() func and freeing
the end result after it was copied into the channel. Since each attempt
would throw away stringfield buffer space (potentially), I wasn't crazy
about using stringfields for this purpose. 

4. Speaking of the name, which is a stringfield, the build func for this
couldn't be called in the channel constructor, but rather in the
channel_alloc() func, so it would be available for the event output.
Since the existing stringfield funcs couldn't build from a va_list, I
invented a new one, and it's the the new heart of the stringfield build
world. It's got a weird calling convention because of bsd's lack of a
va_list copy function. Thank bsd, not me. I'm only the messenger! 

So, I'll be merging this in soon, I hope, after some testing and
lipbiting, and sweating. The possible problems will be, of course: 

1. Newchannel events will get generated more often then previously, and
probably unnecessarily so. Rule of thumb: if you don't want to generate
a newchannel event when you channel_alloc(), pass a null name pointer
for the name format...

If that's not sufficient, best to let me know right away, we'll need yaa
(yet another argument) in the channel_alloc() func to handle this. 

The previous code generated a Newchannel event down in the bowels of the
ast_request function, which could happen who-knows-how-long after the
channel actually got built. Hopefully soon. But for some temp channels,
etc., it may not have ever been called, and apparently nobody shed any
tears over this sort of thing. I noticed that in these sorts of
situations, the channel didn't usually get assigned a name, so I
enshrined this behavior in the channel_alloc calls. So, the temp
channels used in app_voicemail for setting up variables, won't generate
Newchannel events, and neither will the temp channels in pbx.c. But the
parking stuff in res_features assigned a name, and these channels will
show up in the events. 

2. I might have gotten overly zealous in the removal of setstate calls;
we'll have to watch for complaints along this line. But, I was fairly
careful; hopefully few complaints. 

3. I might have been overly zealous in the removal of setcallerid calls,
also. Vigilance is in order. 

Sorry for the longwinded letter. But, oh you channel gurus, beware.
Things have changed just a bit. Thought you ought to know. This is the
kind of situation where the cleanup broom can swat you a good one if you
don't watch out. Since this is a bug, the goal will be to apply it to
1.4 and 1.2 as well as trunk over the next few days.

murf 
-- 
Steve Murphy
Software Developer
Digium
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3227 bytes
Desc: not available
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20061103/fed39e5d/smime.bin