[asterisk-dev] [Code Review] 3346: [res_config_pgsql] Correct handling of nullable int fields in update_realtime

[zvision]

> On March 17, 2014, 9:08 a.m., wdoekes wrote:
> > http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_pgsql.c, lines 761-772
> > <https://reviewboard.asterisk.org/r/3346/diff/1/?file=55850#file55850line761>
> >
> >     The double occurrence of count++ smells bad.
> >     
> >     In both cases we do:
> >     
> >     if (count++) {
> >       ast_str_append(&sql, 0, ", ");
> >     }
> >     
> >     Let's just do that.
> >     
> >     
> >     
> >     As for the column->type strncmp: perhaps that should be inverted to the only cases where "" *is* legal.
> >     Peeking in require_pgsql(), we see this:
> >     
> >       strncmp(column->type, "char", 4) == 0 || strncmp(column->type, "varchar", 7) == 0 || strcmp(column->type, "bpchar") == 0
> >     
> >     Otherwise the bug still exists for the crazy fool who uses a float for a port.

Agreed. I also added a 'text' type (which is very common with PostgreSQL) in the check.
I have uploaded a patch for 1.8 branch.


- zvision


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


On March 17, 2014, 9:58 a.m., zvision wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3346/
> -----------------------------------------------------------
> 
> (Updated March 17, 2014, 9:58 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23351
>     https://issues.asterisk.org/jira/browse/ASTERISK-23351
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch fixes handling of nullable int columns in update_realtime function. It checks if a value is empty and sets the column to NULL instead of '', which raises an error.
> Additionally, it checks for existence of the keyfield column instead of the first parameter column.
> 
> 
> Diffs
> -----
> 
>   http://svn.asterisk.org/svn/asterisk/branches/1.8/res/res_config_pgsql.c 410663 
> 
> Diff: https://reviewboard.asterisk.org/r/3346/diff/
> 
> 
> Testing
> -------
> 
> Only tested for successful compilation. Someone needs to confirm that the patch works fine.
> 
> 
> Thanks,
> 
> zvision
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140317/1bc6b7f2/attachment.html>