[asterisk-dev] [Code Review] High Resolution Call Time for Billsec and Duration

[Russell Bryant]

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


The code looks ok for the most part.  I have a couple of minor comments I have posted.

However, my biggest concern here is the "testing done" section of this review request.  It currently says that compile testing only is what has been done so far.  While that is certainly a starting point, I am absolutely not okay with making changes to the CDR system like this without a detailed test plan, including every backend module that was modified.  Breaking CDRs in any way has a very high cost associated with it since it affects billing systems.


/trunk/addons/cdr_mysql.c
<https://reviewboard.asterisk.org/r/461/#comment3186>

    Is strstr() really what is appropriate here?  I see that it's used in existing code, too, but I'm curious why it's needed instead of strcmp().



/trunk/funcs/func_cdr.c
<https://reviewboard.asterisk.org/r/461/#comment3187>

    Should checking for "billsec" or "duration" be case sensitive here?  Do you intend for "Billsec" and "Duration" to fail?


- Russell


On 2010-01-23 15:57:21, Brad Latus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/461/
> -----------------------------------------------------------
> 
> (Updated 2010-01-23 15:57:21)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This allows for the showing of the accurate times for billable time and call duration.
> Adds new flag to CDR function 'f' for the above fields.
> 
> This came about by bug #16559 and I thought i'd take a go at making it happen.
> 
> We can drop a few of the modules for ones we want to depreciate like sql-lite / odbc (in favour of sql-lite3 / adaptive_odbc)
> 
> 
> This addresses bug 16559.
>     https://issues.asterisk.org/view.php?id=16559
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 240716 
>   /trunk/UPGRADE.txt 240716 
>   /trunk/addons/cdr_mysql.c 240716 
>   /trunk/cdr/cdr_adaptive_odbc.c 240716 
>   /trunk/cdr/cdr_odbc.c 240716 
>   /trunk/cdr/cdr_pgsql.c 240716 
>   /trunk/cdr/cdr_sqlite.c 240716 
>   /trunk/cdr/cdr_tds.c 240716 
>   /trunk/configs/cdr_custom.conf.sample 240716 
>   /trunk/configs/cdr_odbc.conf.sample 240716 
>   /trunk/configs/cdr_sqlite3_custom.conf.sample 240716 
>   /trunk/configs/cdr_syslog.conf.sample 240716 
>   /trunk/configs/cdr_tds.conf.sample 240716 
>   /trunk/funcs/func_cdr.c 240716 
> 
> Diff: https://reviewboard.asterisk.org/r/461/diff
> 
> 
> Testing
> -------
> 
> Compile test only.
> 
> 
> Thanks,
> 
> Brad
> 
>