[asterisk-dev] [Code Review] 3652: pjsip cli: Change Identify to show CIDR notation instead of netmasks.

[Mark Michelson]

> On June 19, 2014, 6:01 p.m., Mark Michelson wrote:
> > branches/12/main/acl.c, line 683
> > <https://reviewboard.asterisk.org/r/3652/diff/1/?file=59869#file59869line683>
> >
> >     It's typically a good idea to avoid ast_strdupa() inside a for loop since it theoretically could blow the stack on a very large list.
> >     
> >     Since addr is const, you don't even need to make a stack copy of it at all.
> 
> George Joseph wrote:
>     This was a cut and paste from the original ast_ha_join.  I removed the strdup here and there don't seem to be any ill effects but I left ast_ha_join alone as it would require more testing.  After all, somebody put it there for a reason. :)

I can answer why ast_ha_join() does it. ast_sockaddr_stringify_* family of functions uses a thread-local buffer to store the stringified result. Each subsequent call to a function in that family will overwrite the buffer with whatever is being stringified. So in that function, the address gets stringified and then the netmask does. The stringified address has to be stored someplace else or else stringifying the netmask will overwrite it. Using ast_strdupa() in ast_ha_join() should probably be changed to use ast_copy_string() into a fixed-sized buffer since IP addresses have an easily-defined maximum size.


- Mark


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


On June 19, 2014, 7:29 p.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3652/
> -----------------------------------------------------------
> 
> (Updated June 19, 2014, 7:29 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> In 'pjsip show endpoints' wouldn't you rather see...
> 
> Identify:  192.168.101.54/32,192.168.147.1/32
> or
> Identify:  26ff:ff:6622:e020::220/128
> 
> instead of...
> 
> Identify:  192.168.101.54/255.255.255.255,192.168.147.1/255.255.255.255
> or
> Identify:  26ff:ff:6622:e020::220/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff
> 
> ?
> 
> I would. :)
> 
> Added ast_sockaddr_cidr_bits() to count the 1 bits in an ast_sockaddr.
> Added ast_ha_join_cidr() which uses ast_sockaddr_cidr_bits() for the netmask instead of ast_sockaddr_stringify_addr.
> Changed res_pjsip_endpoint_identifier_ip to call ast_ha_join_cidr() instead of ast_ha_join().
> 
> This is a CLI change only.  AMI was not affected.
> 
> 
> Diffs
> -----
> 
>   branches/12/res/res_pjsip_endpoint_identifier_ip.c 416729 
>   branches/12/main/netsock2.c 416729 
>   branches/12/main/acl.c 416729 
>   branches/12/include/asterisk/netsock2.h 416729 
>   branches/12/include/asterisk/acl.h 416729 
> 
> Diff: https://reviewboard.asterisk.org/r/3652/diff/
> 
> 
> Testing
> -------
> 
> Made sure both ipv4 and ipv6 addresses were formatted correctly.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140619/dac74736/attachment.html>