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

[Matt Jordan]

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

Ship it!


Two minor points:

(1) You probably don't need to lock the list in each of the callbacks. The only time the PJSIP_HEADER function will be called (or at least, *should* be called) is on the pbx_thread servicing the channel - there shouldn't be any race conditions on those items. That being said, it isn't harmful to lock the list.

(2) There's a few minor formatting things in here, but nothing that can't be cleaned up on check-in.

Nice job!


/branches/12/res/res_pjsip_header_funcs.c
<https://reviewboard.asterisk.org/r/2858/#comment19072>

    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.


- Matt Jordan


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/073b34bc/attachment-0001.html>