[asterisk-dev] [Code Review]: Add #includeif statement

Terry Wilson reviewboard at asterisk.org
Mon Nov 7 09:56:50 CST 2011



On Nov. 3, 2011, 2:49 a.m., Paul Belanger wrote:
> > As for the name -- I would've preferred #require vs. #include, but it's a bit late for that now ;)
> > 
> > The other options:
> > 
> > #sinclude +1
> > #tryinclude +.5
> > #includeif +0
> 
> wdoekes wrote:
>     The reason:
>     
>     I rate the "similar usage in a different domain" (sinclude in m4) higher than self-documentation in this case. (PHP's require/include has them both.)
>     
>     Why? Many people use several languages daily, and having to remember that sinclude is called yinclude in language z is a larger PITA than having to look up the not-self-documenting name, that sinclude unfortunately is, the first time.
> 
> Tilghman Lesher wrote:
>     If we're going down this road, it might be better to make a generic #if/#endif preprocessor entry that allows something like FILEEXISTS() as an argument.  I can see where this would be expanded in the future, and it's better to get the initial syntax correct than to deal with multiple iterations that may need to be grandfathered.

I would be against adding more complexity than is absolutely needed. If someone really needs to do something like #if (it is tuesday) #include tuesday_dialplan.conf, then they need to learn to write a dynamic dialplan, IMHO. There are much better ways to deal with that kind of thing. This is nice and simple--just don't fail on a #include w/o a file. I don't think we should be in the writing-languages-with-preprocessors business.


- Terry


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


On Sept. 30, 2011, 2:56 p.m., Paul Belanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1476/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2011, 2:56 p.m.)
> 
> 
> Review request for Asterisk Developers and leifmadsen.
> 
> 
> Summary
> -------
> 
> Talking with Leif on IRC today, he was asking for a way to include a config file, that might not exist all the time.  Because #include 'foo.conf' to a non existing will fail to load a module, we decided #includeif might be better.
> 
> #exec cat foo.conf is another option, however this seems to require more CPU and opens a potential security issue.
> 
> 
> Diffs
> -----
> 
>   trunk/CHANGES 338717 
>   trunk/main/config.c 338717 
> 
> Diff: https://reviewboard.asterisk.org/r/1476/diff
> 
> 
> Testing
> -------
> 
> Local development box testing.  A few more testers would be nice :D
> 
> 
> Thanks,
> 
> Paul
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111107/cf7f5740/attachment-0001.htm>


More information about the asterisk-dev mailing list