[asterisk-dev] [Code Review] Introduce function for parsing ABNF structure {name-andor-addr = name-addr / addr-spec} in sip messages

[David Vossel]

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

Ship it!


This is the first chance I've had to look over this patch and I really like!  Thanks Nick for taking the initiative to not only see some areas we are lacking in, but actually going and setting it right.  I think the code is ready to go.  I've taken a pass and it seems in order except for a few minor guideline issues.  My only comments have to do with the testing.  I think there are areas were we could have greater coverage without having to necessarily create additional test scenarios.  Look at my comment below and maybe see how that could be applied.  This is a minor complaint.

Thanks again Nick! 


trunk/channels/sip/reqresp_parser.c
<https://reviewboard.asterisk.org/r/549/#comment3863>

    All of the tests in this function have some things in common that could be changed to verify more possibilities.  What if we mixed up having a secret in the URI, or perhaps whether or not the port is set while other parameters are being shifted around.  Doing things like this could give us greater test coverage.


- David


On 2010-03-23 12:06:19, Nick Lewis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/549/
> -----------------------------------------------------------
> 
> (Updated 2010-03-23 12:06:19)
> 
> 
> Review request for Asterisk Developers and David Vossel.
> 
> 
> Summary
> -------
> 
> Many sip headers in many sip methods contain the ABNF structure
>  name-andor-addr = name-addr / addr-spec
> Examples include the to-header, from-header, contact-header, replyto-header, referto-header, referredby-header, and passertedid-header
> 
> At the moment chan_sip.c makes various different attempts to parse this name-andor-addr structure for each header type and for each sip method with sometimes limited degrees of success.
> 
> I here introduce a dedicated function for parsing the name-andor-addr ABNF structure that can be used irrespective of the specific method or header that contains the structure
> 
> (The function is also suited to parsing the name-addr ABNF structure without the addr-spec option. Examples in chan_sip.c include the recordroute-header, route-header, remotepartyid-header and diversion-header.)
> 
> 
> This addresses bug 16708.
>     https://issues.asterisk.org/view.php?id=16708
> 
> 
> Diffs
> -----
> 
>   trunk/channels/sip/include/reqresp_parser.h 249060 
>   trunk/channels/sip/include/sip.h 249060 
>   trunk/channels/sip/reqresp_parser.c 249060 
> 
> Diff: https://reviewboard.asterisk.org/r/549/diff
> 
> 
> Testing
> -------
> 
> Tested using the corresponding unit test function
> 
> 
> Thanks,
> 
> Nick
> 
>