[asterisk-dev] [Code Review] named_acl: Change systemname setting to use the globally set systemname from asterisk.conf

Joshua Colp reviewboard at asterisk.org
Thu Jul 19 10:50:24 CDT 2012


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

Ship it!


The only comment I have is that by having stringfields initialized you will use a bit of memory for no purpose (since none are defined and will never be set). If you expect that to change in the future then fine.

- Joshua


On July 18, 2012, 3:26 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2057/
> -----------------------------------------------------------
> 
> (Updated July 18, 2012, 3:26 p.m.)
> 
> 
> Review request for Asterisk Developers, Olle E Johansson and Matt Jordan.
> 
> 
> Summary
> -------
> 
> oej mentioned on #asterisk-dev about a week or so ago that I had implemented this incorrectly and that I should be using the global systemname option instead. This patch does that and dispenses with the acl.conf option to set it.
> 
> The general section is left in tact for now in spite of the fact that it has no options since getting it working was mildly painful and it's pretty easily conceivable that we'll want to add general options to acl.conf in the near future. Also, if it were removed Asterisk would reject the configuration entirely if a general section were included at all... so that would also be mildly annoying.
> 
> 
> Diffs
> -----
> 
>   /trunk/configs/acl.conf.sample 370226 
>   /trunk/main/named_acl.c 370226 
> 
> Diff: https://reviewboard.asterisk.org/r/2057/diff
> 
> 
> Testing
> -------
> 
> It still compiles and generally works. It also doesn't break my tests.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120719/59e6be95/attachment.htm>


More information about the asterisk-dev mailing list