[asterisk-dev] [svn-commits] murf: branch murf/bug11210 r93535 - in /team/murf/bug11210: channels/ main/

[Steve Murphy]

On Tue, 2007-12-18 at 08:40 +0100, Olle E Johansson wrote:

> 18 dec 2007 kl. 06.50 skrev SVN commits to the Digium repositories:
> 
> > Author: murf
> > Date: Mon Dec 17 23:50:15 2007
> > New Revision: 93535
> >
> > URL: http://svn.digium.com/view/asterisk?view=rev&rev=93535
> > Log:
> > Good. the dialog callids change, hadn't known that. Now I do. Unlink  
> > and relink in the hash table under new name. Sip works better now.
> No, they don't.
> 
> You've ropably discovered a bug. When, exactly does this happen? Do  
> you have a call trace (sip debug) from Asterisk so we
> can fix it ... ???
> 
> Guess we need to find each other on IRC :-)
> 
> /O


It looks pretty purposeful to me. Albeit, I realize that some of these
freshly create a new dialog, and then set the name; this still qualifies
as a name change (to me)-- although it might not to you.   Here's my
enumeration of the places:

In create_addr_from_peer() :

	if (!ast_strlen_zero(peer->fromdomain)) {
		ast_string_field_set(dialog, fromdomain, peer->fromdomain);
		if (!dialog->initreq.headers) {
			char *c;
			char *tmpcall = ast_strdupa(dialog->callid);
			/* this sure looks to me like we are going to change the callid on
this dialog!! */
			c = strchr(tmpcall, '@');
			if (c) {
				*c = '\0';
				ao2_unlink(dialogs,dialog,"About to change the callid -- remove the
old name");
				ast_string_field_build(dialog, callid, "%s@%s", tmpcall,
peer->fromdomain);
				ao2_link(dialogs,dialog,"New dialog callid -- inserted back into
table");
				ast_log(LOG_NOTICE,"=========Changed Dialog name to %s\n",
dialog->callid);
			}
		}
	}
Note: see the ast_string_field_build() call above?

Note2: I'm showing "fixed" code, so in the original, there's no
ao2_unlink, ao2_link calls (of course, since in the original,
astobj2 isn't being used!)


In sip_notify() :

		/* Recalculate our side, and recalculate Call ID */
		ast_sip_ouraddrfor(&p->sa.sin_addr, &p->ourip);
		build_via(p);
		ast_log(LOG_NOTICE,"============About to build callid for %s\n",
p->callid);
		ao2_unlink(dialogs,p,"About to change the callid -- remove the old
name");
		build_callid_pvt(p);
		ao2_link(dialogs,p,"Linking in new name");
		ast_log(LOG_NOTICE,"================New callid: %s\n", p->callid);
		ast_cli(a->fd, "Sending NOTIFY of type '%s' to '%s'\n", a->argv[2],
a->argv[i]);
		dialog_ref(p,"bump the count of p, which transmit_sip_request will
decrement.");
		transmit_sip_request(p, &req);
		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
		dialog_unref(p,"unref pvt at end of for loop in sip_notify");

Note: see the call to build_callid_pvt() above...



In sip_send_mwi_to_peer():

	if (peer->mwipvt) {
		...
	} else {
		/* Build temporary dialog for this message */
		if (!(p = sip_alloc(NULL, NULL, 0, SIP_NOTIFY))) 
			return -1;
		if (create_addr_from_peer(p, peer)) {
                                ...
			return 0;
		}
		/* Recalculate our side, and recalculate Call ID */
		ast_sip_ouraddrfor(&p->sa.sin_addr, &p->ourip);
		ast_log(LOG_NOTICE,"=======================About to build callid for %
s\n", p->callid);
		build_via(p);
		ao2_unlink(dialogs,p,"About to change the callid -- remove the old
name");
		build_callid_pvt(p);
		ao2_link(dialogs,p,"Linking in under new name");
		ast_log(LOG_NOTICE,"====================New callid: %s\n", p->callid);
		/* Destroy this session after 32 secs */
		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
	}

Note: this one is pretty active, as it generates a new dialog, which
sets up a name, and then create_addr_from_peer might modify it, and then
we do a build_callid_pvt to finish up the process!


sip_poke_peer():
	if (!(p = sip_alloc(NULL, NULL, 0, SIP_OPTIONS)))
		return -1;
	peer->call = dialog_ref(p,"copy sip alloc from p to peer->call");
	...

	/* Recalculate our side, and recalculate Call ID */
	ast_log(LOG_NOTICE,"===============About to build callid for %s\n",
p->callid);
	ast_sip_ouraddrfor(&p->sa.sin_addr, &p->ourip);
	build_via(p);
	ao2_unlink(dialogs,p,"About to change the callid -- remove the old
name");
	build_callid_pvt(p);
	ao2_link(dialogs,p,"Linking in under new name");
	ast_log(LOG_NOTICE,"==============New callid: %s\n", p->callid);

Note: again, this routine generates a new dialog via sip_alloc, which
sets the dialog name, and then modifies it...
which in some places might not be classified as a name change, really,
but **I** have consider it such.

sip_alloc() is called in find_call(),  transmit_register(),
sip_notify(), sip_send_mwi_to_peer(), sip_poke_peer(), and
sip_request_call(), (The names in red are mentioned above). So, the
routines above are not the only ones to allocate a dialog, and not all
functions that allocate a dialog reset the name.

murf


-- 
Steve Murphy
Software Developer
Digium
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.digium.com/pipermail/asterisk-dev/attachments/20071218/c12b2165/attachment.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3239 bytes
Desc: not available
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20071218/c12b2165/attachment.bin