[asterisk-dev] [Code Review] 2858: res_pjsip_header_funcs: New module to create PJSIP_HEADER, PJSIPAddHeader and PJSIPRemoveHeader

[Joshua Colp]

> On Oct. 3, 2013, 6:33 p.m., Matt Jordan wrote:
> > /branches/12/res/res_pjsip_header_funcs.c, lines 54-60
> > <https://reviewboard.asterisk.org/r/2858/diff/3/?file=46938#file46938line54>
> >
> >     It may be worthwhile adding a <note> to this that states that reading PJSIP_HEADER with the remove option will still remove the headers. That can be done before commit however.
> 
> George Joseph wrote:
>     Just confirming... the same thread that PJSIP_HEADER will be called from will also call the supplemental session callbacks?  If you allow me to say "Matt said so" I'll remove the locks. :)
>     
>     Since I don't have commit access, I'll add the note, fix the formatting and re-upload the patch.  I ran indent and didn't see anything obvious in the formatting so let me know what should be fixed.

Oh now I understand why you lock. Locking in the PJSIP architecture is so uncommon. Yeah, this code may work as-is but it's not obeying the approach set out. Operations exist as tasks which are pushed onto a task queue for the session itself. This ensures that operations for the session are serialized, and also guarantees that multiple things aren't operating on a session at the same time. media_offer_write would be a good thing to use as a basis.


- Joshua


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


On Sept. 30, 2013, 9:15 p.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2858/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2013, 9:15 p.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Bugs: ASTERISK-22498
>     https://issues.asterisk.org/jira/browse/ASTERISK-22498
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> For PJSIP_HEADER, an incoming supplemental session callback is registered that takes the pjsip_hdrs from the incoming session and stores them in a linked list in the session datastore.  Calls to PJSIP_HEADER traverse over the list and return the nth matching header where 'n' is the 'number' argument to the function.
> 
> For PJSIPAddHeader, the first call creates a datastore and linked list and adds the datastore to the session.  The header is then created as a pjsip_hdr and added to the list.  An outgoing supplemental session callback then traverses the list and adds the headers to the outgoing pjsip_msg.
> 
> For PJSIPRemoveHeader, the list created with PJSIPAddHeader is traversed and all matching entries are removed.  As with SIPRemoveHeader, an empty arguments removes all headers previously added.
> 
> Rather than cloning the incoming pjsip_msg or using pjsip_msg to accumulate the outgoing headers, I used a simple AST_LIST. There was a lot of overhead with the clone functions and some tricky behavior with the pjsip_msg/pjsip_hdr internal lists. Using the AST_LISTs cut down on both instructions and memory.  
> 
> All memory allocation is from the pj_pool attached to the session.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_header_funcs.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/2858/diff/
> 
> 
> Testing
> -------
> 
> Tested successfully...
> PJSIP_HEADER return the full value of nth specified header from either the incoming session, or headers previously added to the outgoing session by PJSIPHeader.
> PJSIP_HEADER fails if there was no header specified in the function call.
> PJSIP_HEADER fails if there was no datastore on the incoming session (should never happen).
> PJSIP_HEADER fails if the nth header wasn't found.
> 
> PJSIPAddHeader adds a pjsip_hdr structure to the linked list when the input string is properly formatted as "header_name:\s*header_value".
> PJSIPAddHeader fails if no ':' was found in the input string.
> PJSIPAddHeader fails if after parsing either the header name or value is empty.
> 
> PJSIPRemoveHeader removes all matching headers from the linked list when a partial header name is specified.
> PJSIPRemoveHeader removes all matching headers from the linked list when a full header is specified with a trailing ':'.
> PJSIPRemoveHeader removes all previously added header from the linked list when no header is specified.
> PJSIPRemoveHeader returns successfully (silently) if there was no linked list.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131003/4a311068/attachment.html>