Hi Daniel,
Could you please review this patch? It is the last one from the achemeris/sms-validity series and is the smallest one.
On Sat, Mar 8, 2014 at 12:24 AM, Alexander Chemeris alexander.chemeris@gmail.com wrote:
-- Regards, Alexander Chemeris. CEO, Fairwaves, Inc. / ООО УмРадио https://fairwaves.co
Hi Alexander,
On Sun, 2014-03-16 at 13:35, Alexander Chemeris wrote:
Could you please review this patch? It is the last one from the achemeris/sms-validity series and is the smallest one.
this patch looks fine.
Regards, Daniel
Dear Alexander,
On Fri, 2014-03-07 at 21:24, Alexander Chemeris wrote:
-- Regards, Alexander Chemeris. CEO, Fairwaves, Inc. / ООО УмРадио https://fairwaves.co
From 44600bbe8d0de88b7fbb2530e12b9122bf6e52df Mon Sep 17 00:00:00 2001 From: Alexander Chemeris Alexander.Chemeris@gmail.com Date: Fri, 7 Mar 2014 21:02:46 +0100 Subject: [PATCH 5/6] sms: Fix support of negative timezone offsets in gsm340_gen_scts().
src/gsm/gsm0411_utils.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/gsm/gsm0411_utils.c b/src/gsm/gsm0411_utils.c index 6272ba3..e884eca 100644 --- a/src/gsm/gsm0411_utils.c +++ b/src/gsm/gsm0411_utils.c @@ -85,7 +85,10 @@ void gsm340_gen_scts(uint8_t *scts, time_t time) *scts++ = gsm411_bcdify(tm->tm_min); *scts++ = gsm411_bcdify(tm->tm_sec); #ifdef HAVE_TM_GMTOFF_IN_TM
- *scts++ = gsm411_bcdify(tm->tm_gmtoff/(60*15));
- if (tm->tm_gmtoff >= 0)
*scts++ = gsm411_bcdify(tm->tm_gmtoff/(60*15));- else
*scts++ = gsm411_bcdify(-tm->tm_gmtoff/(60*15)) | 0x80;#else #warning find a portable way to obtain timezone offset
*scts++ = 0;
1.7.9.5
after looking through the code and thinking about mktime/timegm some more I noticed that we actually want to used localtime() instead of gmtime() in this function. gmtime will always set tm_gmtoff zero (as it converts it into UTC).
This function could be written without any use of tm_gmtoff by using both localtime and gmtime, converting these results back to time_t with mktime and calculating the difference.
I'll send a patch in a an extra mail.
Regards, Daniel
The current code uses gmtime which returns the time in UTC. This is not the intended behaviour (otherwise the offset could always be set to zero).
This uses localtime as well as gmtime to calculate the timezone offset instead of relying on tm_gmtoff. This means three extra function calls (mktime * 2, gmtime_r), but this is completely independent of non-standard features. --- src/gsm/gsm0411_utils.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/src/gsm/gsm0411_utils.c b/src/gsm/gsm0411_utils.c index bb59a10..18664dc 100644 --- a/src/gsm/gsm0411_utils.c +++ b/src/gsm/gsm0411_utils.c @@ -77,23 +77,32 @@ uint8_t gsm411_unbcdify(uint8_t value) /* Generate 03.40 TP-SCTS */ void gsm340_gen_scts(uint8_t *scts, time_t time) { - struct tm *tm = gmtime(&time); - - *scts++ = gsm411_bcdify(tm->tm_year % 100); - *scts++ = gsm411_bcdify(tm->tm_mon + 1); - *scts++ = gsm411_bcdify(tm->tm_mday); - *scts++ = gsm411_bcdify(tm->tm_hour); - *scts++ = gsm411_bcdify(tm->tm_min); - *scts++ = gsm411_bcdify(tm->tm_sec); -#ifdef HAVE_TM_GMTOFF_IN_TM - if (tm->tm_gmtoff >= 0) - *scts++ = gsm411_bcdify(tm->tm_gmtoff/(60*15)); + struct tm tm_local, tm_utc; + time_t ts_local, ts_utc, gmtoffset; + + localtime_r(&time, &tm_local); + + *scts++ = gsm411_bcdify(tm_local.tm_year % 100); + *scts++ = gsm411_bcdify(tm_local.tm_mon + 1); + *scts++ = gsm411_bcdify(tm_local.tm_mday); + *scts++ = gsm411_bcdify(tm_local.tm_hour); + *scts++ = gsm411_bcdify(tm_local.tm_min); + *scts++ = gsm411_bcdify(tm_local.tm_sec); + + /* Figure out the timezone offset in a portable way. + * The idea is to convert the time_t into local and UTC struct tm + * representations and then calculate the difference of both. */ + gmtime_r(&time, &tm_utc); + tm_utc.is_dst = 0; + tm_local.is_dst = 0; + ts_utc = mktime(&tm_utc); + ts_local = mktime(&tm_local); + gmtoffset = ts_local - ts_utc; + + if (gmtoffset >= 0) + *scts++ = gsm411_bcdify(gmtoffset/(60*15)); else - *scts++ = gsm411_bcdify(-tm->tm_gmtoff/(60*15)) | 0x80; -#else -#warning find a portable way to obtain timezone offset - *scts++ = 0; -#endif + *scts++ = gsm411_bcdify(-gmtoffset/(60*15)) | 0x80; }
/* Decode 03.40 TP-SCTS (into utc/gmt timestamp) */
-
Alexander Chemeris -
Daniel Willmann