[asterisk-dev] [Code Review] Iterate though cdr.conf setting

[Tilghman Lesher]

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


I don't think we've specifically agreed that this is the "proper way" to parse configuration files, only that this way is a more efficient use of CPU.  In fact, there is one configuration file across all modules that consistently does it the opposite way, which is users.conf.  If users.conf is primarily edited by machine, this shouldn't be a problem, since the web process already eliminates duplicate values.  However, if others are editing users.conf, this has the potential to blow up, especially given that people who use users.conf tend to be our most helpless user population.


trunk/main/cdr.c
<https://reviewboard.asterisk.org/r/1426/#comment8375>

    I'd like these all to be extended as a continuous 'else if'.  This saves considerable CPU, as once one of them matches, the loop can short-circuit, instead of needlessly comparing against more values (that we know won't match).


- Tilghman


On Sept. 10, 2011, 2:57 p.m., Paul Belanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1426/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2011, 2:57 p.m.)
> 
> 
> Review request for Asterisk Developers and leifmadsen.
> 
> 
> Summary
> -------
> 
> We had some internal discussions this week about how to update configuration files so they worked the same across all modules.  In this example, I've updated cdr.conf to use the same method of iterating over all .conf file variables.  Most people agreed this was the proper way to parse a configuration file.
> 
> If we all agreed, I'd like this method to become the standard way used for parsing configuration file.  Obviously we have some questions that need to be answered, specifically this _may_ break existing configuration files if a duplicate variable is declared later in a .conf file (AKA unused garbage settings).
> 
> FWIW: This is how we do it for manager.conf, features.conf, sip.conf, plus various other modules.
> 
> Thoughts? 
> 
> 
> Diffs
> -----
> 
>   trunk/main/cdr.c 335124 
> 
> Diff: https://reviewboard.asterisk.org/r/1426/diff
> 
> 
> Testing
> -------
> 
> Local asterisk box.
> 
> 
> Thanks,
> 
> Paul
> 
>

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