Re: FW: Validity Period wrong calculation in SMPP code

Alexander Malysh <[email protected]> Wed, 17 Nov 2004 13:24:43 +0100

Newsgroups	gmane.comp.mobile.kannel.devel
Message-ID	<[email protected]>

Hi,

good catch, seems to be a kannel bug...
Please try attached patch whether then a bug fixed?

Btw. I don't know, why we calculate local <-> utc diff and for what it
should be good? Patch just set timestamps in utc...

Hillel Bilman wrote:

> 
> Is this correct?
> -----Original Message-----
> From: [email protected] [mailto:[email protected]]On
> Behalf Of Roberto Carlos Navas
> Sent: Sunday, November 14, 2004 4:07 PM
> To: [email protected]
> Subject: Validity Period wrong calculation in SMPP code
> 
> 
> I know this is not the place, but I didn't want to subscribe to the
> developers list... (i'm lazy)
> I found a problem with the way the local timezone is determined in the
> SMPP code... this timezone is used to calculate the validity period
> field in the PDU.
> The current method for calculation is getting the localtime and then
> the GMTtime... doing a substraction and then getting the difference in
> hours... the problem arise when the localtime is let's say: 20:00 and
> GMTtime is 02:00 the next day... the expected result should be GMT-6.
> Here is how I solved it on Linux... don't know how portable is the code
> to other platforms... but the developers will find a better solution:
> 
> diff -r gateway-1.3.2/gw/smsc/smsc_smpp.c
> gateway-1.3.2-telemovil/gw/smsc/smsc_smpp.c
> 539a540,542
>  >     extern long timezone;
>  >
>  >     tzset();
> 690a694,696
>  > /*    The following formula to find out the local timezone is wrong
>  >       the test case is when localtime is 20:00 and timezone is GMT-6
>  >
> 692a699,700
>  > */
>  >       gwqdiff = ((timezone / 900) * -1);
> 
> Regards,
> ______________________________________________
> Roberto Carlos Navas
> SubGerente TÃ©cnico de Internet
> Telemovil El Salvador
> Centro Financiero Gigante, Torre "D", Piso 9
> San Salvador, El Salvador
> Mobile: +503 8832010
> NOC: +503 2080008
> Fax: +503 2809450

-- 
Thanks,
Alex

smsc-smpp-validity-deferred.patch (text/x-diff, 3.3 KB)

Index: gw/smsc/smsc_smpp.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsc/smsc_smpp.c,v
retrieving revision 1.75
diff -a -u -p -r1.75 smsc_smpp.c
--- gw/smsc/smsc_smpp.c	21 Oct 2004 12:33:40 -0000	1.75
+++ gw/smsc/smsc_smpp.c	17 Nov 2004 12:19:43 -0000
@@ -535,10 +535,6 @@ static long smpp_status_to_smscconn_fail
 static SMPP_PDU *msg_to_pdu(SMPP *smpp, Msg *msg)
 {
     SMPP_PDU *pdu;
-    Octstr *buffer;
-    Octstr *relation_UTC_time = NULL;
-    struct tm gmtime, localtime, tm;
-    int gwqdiff;
 
     pdu = smpp_pdu_create(submit_sm,
     	    	    	  counter_increase(smpp->message_id_counter));
@@ -685,40 +681,18 @@ static SMPP_PDU *msg_to_pdu(SMPP *smpp, 
     /*
      * check for validity and defered settings
      */
-    if (msg->sms.validity >= 0 || msg->sms.deferred >= 0) {
-
-        /* work out 1/4 hour difference between local time and UTC/GMT */
-        gmtime = gw_gmtime(time(NULL));
-        localtime = gw_localtime(time(NULL));
-        gwqdiff = ((localtime.tm_hour - gmtime.tm_hour) * 4)
-                  + ((localtime.tm_min - gmtime.tm_min) / 15);
-
-        if (gwqdiff >= 0) {
-            relation_UTC_time = octstr_create("+");
-        } else {
-            relation_UTC_time = octstr_create("-");
-            gwqdiff *= -1;  /* make absolute */
-        }
-
-        if (msg->sms.validity >= 0) {
-            tm = gw_localtime(time(NULL) + msg->sms.validity * 60);
-            buffer = octstr_format("%02d%02d%02d%02d%02d%02d0%02d%1s",
-                    tm.tm_year % 100, tm.tm_mon + 1, tm.tm_mday,
-                    tm.tm_hour, tm.tm_min, tm.tm_sec,
-                    gwqdiff, octstr_get_cstr(relation_UTC_time));
-            pdu->u.submit_sm.validity_period = octstr_copy(buffer,0,16);
-            octstr_destroy(buffer);
-        }
+    if (msg->sms.validity >= 0) {
+        struct tm tm = gw_gmtime(time(NULL) + msg->sms.validity * 60);
+        pdu->u.submit_sm.validity_period = octstr_format("%02d%02d%02d%02d%02d%02d000+",
+                tm.tm_year % 100, tm.tm_mon + 1, tm.tm_mday,
+                tm.tm_hour, tm.tm_min, tm.tm_sec);
+    }
 
-        if (msg->sms.deferred >= 0) {
-            tm = gw_localtime(time(NULL) + msg->sms.deferred * 60);
-            buffer = octstr_format("%02d%02d%02d%02d%02d%02d0%02d%1s",
-                    tm.tm_year % 100, tm.tm_mon + 1, tm.tm_mday,
-                    tm.tm_hour, tm.tm_min, tm.tm_sec,
-                    gwqdiff, octstr_get_cstr(relation_UTC_time));
-            pdu->u.submit_sm.schedule_delivery_time = octstr_copy(buffer,0,16);
-            octstr_destroy(buffer);
-        }
+    if (msg->sms.deferred >= 0) {
+        struct tm tm = gw_gmtime(time(NULL) + msg->sms.deferred * 60);
+        pdu->u.submit_sm.schedule_delivery_time = octstr_format("%02d%02d%02d%02d%02d%02d000+",
+                tm.tm_year % 100, tm.tm_mon + 1, tm.tm_mday,
+                tm.tm_hour, tm.tm_min, tm.tm_sec);
     }
 
     /* ask for the delivery reports if needed */
@@ -727,8 +701,6 @@ static SMPP_PDU *msg_to_pdu(SMPP *smpp, 
     else if (DLR_IS_FAIL(msg->sms.dlr_mask) && !DLR_IS_SUCCESS(msg->sms.dlr_mask))
         pdu->u.submit_sm.registered_delivery = 2;
 
-    octstr_destroy(relation_UTC_time);
-
     /* set priority */
     if (msg->sms.priority >= 0 && msg->sms.priority <= 3)
         pdu->u.submit_sm.priority_flag = msg->sms.priority;