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

[George Joseph]

> On Sept. 17, 2013, 1:22 p.m., Mark Michelson wrote:
> > /branches/12/res/res_pjsip_header_funcs.c, lines 164-175
> > <https://reviewboard.asterisk.org/r/2858/diff/1/?file=45986#file45986line164>
> >
> >     The use of rdata->tp_info.pool here worries me. I think that it would be better to use a pool whose lifetime is the same scope as the ast_sip_session. On the dev-list, I pointed out a way to get to the pjsip_dialog's pool. That would be a better choice here. Either that, or the pjsip_inv_session's pool_active.

Also copied from another module. :)  Do you think I should use the inv_session->pool_active or go all the way down to dlg->pool?  


> On Sept. 17, 2013, 1:22 p.m., Mark Michelson wrote:
> > /branches/12/res/res_pjsip_header_funcs.c, line 373
> > <https://reviewboard.asterisk.org/r/2858/diff/1/?file=45986#file45986line373>
> >
> >     Instead of checking length and using pj_strnicmp2(), just use pjstricmp()

I think that because the ':' is part of the supplied value but not part of the header itself I'd have to strdup 'data' and overwrite the ':' with \0 to use a stricmp.  I was just trying to save the copy.


- George


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


On Sept. 15, 2013, 6:34 p.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2858/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2013, 6:34 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/20130917/5cff0e5f/attachment-0001.htm>