> On Jan. 16, 2015, 8:19 a.m., wdoekes wrote: > >
Thanks for the quick feedback! > On Jan. 16, 2015, 8:19 a.m., wdoekes wrote: > > /branches/11/channels/chan_sip.c, lines 13752-13755 > > <https://reviewboard.asterisk.org/r/4346/diff/1/?file=70621#file70621line13752> > > > > I'd rather see this code moved/duplicated into uas_sips_contact and > > uac_sips_contact. Right now it looked like you missed half the criteria > > when looking at those functions. I initially did that and then factored the common code out (because of DRY and all). But if it's more clear having the request URI checked in uas_sips_contact and uac_sips_contact, I can make the change. > On Jan. 16, 2015, 8:19 a.m., wdoekes wrote: > > /branches/11/channels/chan_sip.c, lines 14378-14380 > > <https://reviewboard.asterisk.org/r/4346/diff/1/?file=70621#file70621line14378> > > > > Can this be safely removed? > > > > Does the contact get added somewhere else? > > Is there not a reason why it's done right after the ast_sip_ouraddrfor? >From tracing this by hand, I could not see why this was here. build_contact() >is called here in order to build the contact header on the mwi->call sip_pvt. >Later, transmit_invite() is called with its init parameter set to 2, meaning >that initreqprep will be called. At the end of initreqprep, the contact is >rebuilt on the sip_pvt (mwi->call), overwriting what was previously created. Between this call to build_contact() and the one in initreqprep(), I could find no references to the sip_pvt's contact field being used. This adds up to this call to build_contact() being a no-op. My suspicion is that this is the result of "organic" code growth in chan_sip. At one point, this may have been needed but it no longer is. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4346/#review14209 ----------------------------------------------------------- On Jan. 15, 2015, 8:07 p.m., Mark Michelson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4346/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2015, 8:07 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24646 > https://issues.asterisk.org/jira/browse/ASTERISK-24646 > > > Repository: Asterisk > > > Description > ------- > > Please see the description on /r/4345 for a description of the > interoperability problem. > > This patch is the chan_sip version of the patch. The chan_sip patch is a bit > more weird since creation of Contact headers is not divided into dialog vs. > non-dialog situations. For this patch, I have expanded the build_contact() > function to base the scheme of the Contact header based on the rules of RFC > 3261 sections 8.1.1.8 and 12.1.1. To do this, I have added two parameters so > that it is clear whether the Contact header is being created due to a > response to an incoming request or if the Contact is being added to a created > outgoing request. > > > Diffs > ----- > > /branches/11/channels/chan_sip.c 428405 > > Diff: https://reviewboard.asterisk.org/r/4346/diff/ > > > Testing > ------- > > This patch has been tested by the reporter of ASTERISK-24646 and has been > confirmed to solve the interop issue. > > > Thanks, > > Mark Michelson > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
