[asterisk-dev] [Code Review] SIP: more code refactoring, more unit tests

[Nick Lewis]

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

Ship it!


Minor comments


/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/499/#comment3373>

    if (get_name_and_number(...)) {
     ast_log(...failed parsing to-header...)
    }
    



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

    How about tmp_name[50] and *tmp_number as hdr may not be a to-header



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

    change "to header" to just "header"
    
    The calling function can log a message that includes the header type if it sees a retval of -1


- Nick


On 2010-02-11 10:42:22, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/499/
> -----------------------------------------------------------
> 
> (Updated 2010-02-11 10:42:22)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> ----- Code Refactoring Changes ----
> - read_to_parts() moved to reqresp_parser.c and has been renamed as get_name_and_number()
> - get_in_brackets() moved to reqresp_parser.c
> - find_closing_quotes() added to sip_utils.h
> 
> ----- Logic Changes -----
> - get_name_and_number() now uses parse_uri() and get_calleridname() for parsing.  Before this change only names within quotes were found, when names not within quotes are possible.
> 
> ----- New Unit Tests -----
> sip_get_name_and_number_test
> sip_get_in_brackets_test
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 246298 
>   /trunk/channels/sip/include/reqresp_parser.h 246298 
>   /trunk/channels/sip/include/sip_utils.h 246298 
>   /trunk/channels/sip/reqresp_parser.c 246298 
> 
> Diff: https://reviewboard.asterisk.org/r/499/diff
> 
> 
> Testing
> -------
> 
> -tested sip redirect to verify get_name_and_number still works as expected.
> -ran unit tests, verified they all passed.
> 
> 
> Thanks,
> 
> David
> 
>