[asterisk-dev] [Code Review] 3591: application_name support in cdr_pgsql and res_config_pgsql

[wdoekes]

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


Thanks for the feature.

This will also require updates to:

- CHANGES.txt <-- new features
- configs/*pgsql.conf <-- example configs



/trunk/cdr/cdr_pgsql.c
<https://reviewboard.asterisk.org/r/3591/#comment22079>

    Please split this into multiple lines.
    
    I know it was like that before, but since you're touching it, you can fix it :)



/trunk/cdr/cdr_pgsql.c
<https://reviewboard.asterisk.org/r/3591/#comment22080>

    No camelCase please.
    
    conn_info



/trunk/cdr/cdr_pgsql.c
<https://reviewboard.asterisk.org/r/3591/#comment22082>

    s/pgdbappname/pgappname/



/trunk/cdr/cdr_pgsql.c
<https://reviewboard.asterisk.org/r/3591/#comment22081>

    Remove these debug statements.
    
    - We don't want passwords in the log if it can be avoided.
    - We certainly don't need the debug message twice.



/trunk/cdr/cdr_pgsql.c
<https://reviewboard.asterisk.org/r/3591/#comment22083>

    No need for this debug statement either.



/trunk/cdr/cdr_pgsql.c
<https://reviewboard.asterisk.org/r/3591/#comment22084>

    - Remove the warning, one shouldn't be required to use this.
    
    - For consistency with the surrounding code, add a blank before the ast_free.



/trunk/cdr/cdr_pgsql.c
<https://reviewboard.asterisk.org/r/3591/#comment22085>

    - The connInfo declaration should be at the top of the function.
    
    - And the same comments as above apply here.
    
    - And since this is now duplicate code, I suggest you move it into a single static pg_connect() function.



/trunk/res/res_config_pgsql.c
<https://reviewboard.asterisk.org/r/3591/#comment22087>

    dbappname[0] = '\0';
    
    Try to avoid using strcpy, even when it is safe. And in this case, the above is quicker anyway.
    


- wdoekes


On June 6, 2014, 11:23 a.m., doome wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3591/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 11:23 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23737
>     https://issues.asterisk.org/jira/browse/ASTERISK-23737
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> See https://issues.asterisk.org/jira/browse/ASTERISK-23737
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_config_pgsql.c 415300 
>   /trunk/cdr/cdr_pgsql.c 415300 
> 
> Diff: https://reviewboard.asterisk.org/r/3591/diff/
> 
> 
> Testing
> -------
> 
> Used asterisk with the config paramterer pgdbappname and dbappname in cdr_pgsql.conf and res_config_pgsql.conf respectively, enabled and disabled. Worked as expected.
> 
> 
> Thanks,
> 
> doome
> 
>

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