[Asterisk-code-review] app voicemail: Fix data-type mismatch between app voicemail ... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/8681 )

Change subject: app_voicemail: Fix data-type mismatch between app_voicemail and database
......................................................................


Patch Set 4:

> > I don't think converting the string to an int is the right
 > > approach.  The origtime is a timestamp which has a varying
 > integer
 > > size type depending upon the machine.  The time_t type could be
 > > "long int" or "long long int".
 > >
 > > In ASTERISK-27760, you said the MariaDB had a problem with the
 > > empty string and not a number string.  It seems that the MariaDB
 > is
 > > treating the duration and origtime columns as "INTEGER NOT NULL"
 > > instead of "INTEGER" or "INTEGER NULL" type columns.  To avoid
 > type
 > > conversion problems we should instead make sure that the duration
 > > and origtime strings are not empty in store_file() right before
 > the
 > > call to ast_odbc_direct_execute().  If they are empty then we can
 > > substitute the "0" string instead.
 > >
 > > if (ast_strlen_zero(idata.origtime)) {
 > > idata.origtime = "0";
 > > }
 > > if (ast_strlen_zero(idata.duration)) {
 > > idata.duration = "0";
 > > }
 > 
 > Good point on the time_t being a long on some machines. I guess if
 > this is the approach we should update the database schema also in
 > /contrib/realtime/mysql_voicemail.sql since its specified as an
 > integer in there (which is why I chose an int for the conversion in
 > the first place). Both approaches (int to DB and string to DB) will
 > start overflowing peoples database integer in 2038.
 > I'm on holiday now for a while. Ill try and look at these changes
 > when I return.
 > Cheers, Nic.

I think it is the database implementation that determines the maximum values allowed by integer columns.


-- 
To view, visit https://gerrit.asterisk.org/8681
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I8543ad480a08c98be78bde1ee870e6e6c84b2c5b
Gerrit-Change-Number: 8681
Gerrit-PatchSet: 4
Gerrit-Owner: Nic Colledge <nic at njcolledge.net>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Nic Colledge <nic at njcolledge.net>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 22:40:25 +0000
Gerrit-HasComments: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180410/36b044d9/attachment-0001.html>