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

Ship it!


A couple of minor nitpicks, but other than that it looks good to go.


/branches/12/res/res_pjsip_endpoint_identifier_ip.c
<https://reviewboard.asterisk.org/r/3995/#comment23810>

    Since you switched from the goto usage from your first diff, you can go 
back to not initializing addrs.



/branches/12/res/res_pjsip_endpoint_identifier_ip.c
<https://reviewboard.asterisk.org/r/3995/#comment23809>

    Print current_string instead of var->value here.


- Mark Michelson


On Sept. 17, 2014, 10:29 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3995/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 10:29 p.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Bugs: ASTERISK-24290
>     https://issues.asterisk.org/jira/browse/ASTERISK-24290
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Using a value such as '10.24.0.0/16' would fail to match because 
> ast_sockaddr_resolve can only parse the following formats:
> 
>  * hostname:port
>  * host.example.com:port
>  * a.b.c.d
>  * a.b.c.d:port
>  * a:b:c:...:d
>  * [a:b:c:...:d]
>  * [a:b:c:...:d]:port
> 
> When the format doesn't match one of these, the function fails and we bail.
> 
> To get around this, I simply checked for the presence of a '/' in the 
> identify string and used ast_append_ha directly with the address if it was 
> present.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_endpoint_identifier_ip.c 423062 
> 
> Diff: https://reviewboard.asterisk.org/r/3995/diff/
> 
> 
> Testing
> -------
> 
> Used CLI command 'pjsip show endpoint 1603' with an endpoint that had the 
> following identifier:
> 
> [1603]
> type=identify
> match=10.24.18.13/16
> endpoint=1603
> 
> 
> Before, the address would fail to parse and the command would show no 
> identifier
> After, the address would parse correctly and show '10.24.0.0/16' for the 
> identifier as seen in:
> 
>  Endpoint:  1603/1603                                            Not in use   
>  0 of inf
>         Aor:  1603                                               5
>       Contact:  1603/sip:[email protected]:5060;ob                Unknown      
>          nan
>    Identify:  10.24.0.0/16
> 
> I tried a few other things, such as not using a CIDR and using a hostname to 
> verify that there wasn't any obvious deviation in behavior introduced by the 
> patch.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to