[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] [Code Review] Extend pvt->relatedpeer support to
From:       "Nick Lewis" <Nick.Lewis () atltelecom ! com>
Date:       2010-04-29 13:16:45
Message-ID: 20100429131645.7976.95760 () hotblack ! digium ! internal
[Download RAW message or body]



> On 2010-04-28 14:39:17, Mark Michelson wrote:
> > This is a type of change I can get behind. Many times, it is a complete pain to \
> > be in the middle of a call and have to call find_peer in order to find the \
> > associated peer for a dialog (see the handling of 405 and 501 responses in \
> > handle_response() for an example). 
> > I think that this is a good first step. The problem is that in its current guise, \
> > it's not really adding any benefit. You've made TODO comments in a couple of \
> > places, which means you at least recognize where improvements can be made. I \
> > recommend making those improvements and posting them. That way, we can get it all \
> > in one go and not have any lingering responsibilities. 
> > By the way, I see absolutely no issues with the code as written, just to be \
> > clear.
> 
> pprindeville wrote:
> "Not really adding any benefit"?  Well, it allows me to commit #565... that's a \
> start. 
> Can we get a "ship it!"?
> 

I guess that to take full advantage of pvt->relatedpeer would require much of \
chan_sip.c to be rewritten. I had anticipated that new functionality would make use \
of it initially (such as callbackexten matching) before optimising existing code to \
remove unnecessary searches and duplicated data. As a compromise perhaps I could \
optimise one part of the existing code to show some small immediate benefit?


- Nick


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/629/#review1915
-----------------------------------------------------------


On 2010-04-27 11:14:43, Nick Lewis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/629/
> -----------------------------------------------------------
> 
> (Updated 2010-04-27 11:14:43)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and pprindeville.
> 
> 
> Summary
> -------
> 
> There is an item (relatedpeer) in the pvt structure that references a related \
> (matched) peer structure. This is used during processing of SUBSCRIBE, NOTIFY, and \
> OPTIONS requests and permits peer configuration items to be used without \
> duplicating them in the pvt structure. The attached change extends the availability \
> of the relatedpeer to INVITE processing too. 
> 
> Diffs
> -----
> 
> trunk/channels/chan_sip.c 258227 
> 
> Diff: https://reviewboard.asterisk.org/r/629/diff
> 
> 
> Testing
> -------
> 
> During a incoming call the matched peer appears correctly in a new "Peer" field in \
> the output to the "sip show channels" cli command 
> 
> Thanks,
> 
> Nick
> 
> 


-- 
_____________________________________________________________________
-- 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


[prev in list] [next in list] [prev in thread] [next in thread]