On Wed, Mar 12, 2014 at 10:53 PM, Daniel Willmann <dwillmann(a)sysmocom.de> wrote:
Hello Alexander,
On Fri, 2014-03-07 at 21:25, Alexander Chemeris wrote:
@@ -100,11 +100,13 @@ void
gsm340_gen_scts(uint8_t *scts, time_t time)
time_t gsm340_scts(uint8_t *scts)
{
struct tm tm;
- uint8_t yr = gsm411_unbcdify(*scts++);
- int ofs;
+ uint8_t yr, tz;
+ int ofs = 0;
Do we need to initialize ofs? It looks like both before and now ofs is
always assigned before use.
Yeah, a leftover from an intermediate implementation I had. Fixed.
@@ -114,15
+116,28 @@ time_t gsm340_scts(uint8_t *scts)
tm.tm_hour = gsm411_unbcdify(*scts++);
tm.tm_min = gsm411_unbcdify(*scts++);
tm.tm_sec = gsm411_unbcdify(*scts++);
-#ifdef HAVE_TM_GMTOFF_IN_TM
- tm.tm_gmtoff = gsm411_unbcdify(*scts++) * 15*60;
-#endif
By deleting the #ifdef here and not adding it below you'll break cygwin
builds. See commit 7c8e2cc7aca1a789bbcf989a14be177d59041959.
Fixed.
/*
according to gsm 03.40 time zone is
"expressed in quarters of an hour" */
- ofs = gsm411_unbcdify(*scts++) * 15*60;
-
- return mktime(&tm) - ofs;
+ tz = *scts++;
+ ofs = gsm411_unbcdify(tz&0x7f) * 15*60;
+ if (tz&0x80)
+ ofs = -ofs;
+ /* mktime() doesn't tm.tm_gmtoff into account. Instead, it fills this field
doesn't take
Thanks.
+ * with
the current timezone. Which means that the resulting time is off
+ * by several hours after that. So here we're setting tm.tm_isdt to -1
+ * to indicate that the tm time is local, but later we subtract the
+ * offset introduced by mktime. */
+ tm.tm_isdst = -1;
+
+ timestamp = mktime(&tm);
+ if (timestamp < 0)
+ return -1;
+
+ /* Subtract artificial timezone offset, introduced by mktime() */
+ timestamp = timestamp - ofs + tm.tm_gmtoff;
+
+ return timestamp;
}
/* Decode validity period format 'relative'.
--
1.8.4.2
I'm still not convinced that we want to use mktime and tm_gmtoff for the
calculation. There's timegm() which is the inverse of gmtime() which I
think we should use were it's available. Depending on timegm shouldn't
make us and more dependent on some extension than we already are with
tm_gmtoff.
The man page has a note on how to implement a portable version, though
it involves calling setenv (to set TZ to UTC) and I'm not sure we want
to do that in a library.
Additional discussion welcome.
I spent some time thinking about the best solution and came to
conclusion that the one with mktime() is the best one.
gmtime() is a non-standard extension, so we'll have to detect it, add
#ifdef's and then we still will need a code which works if it is not
available. And that code will be based on mktime(). So why not to use
mktime() from the very beginning?
The code is covered with tests, so we will notice any breakage if it
occurs on other platforms.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves, Inc. / ООО УмРадио
https://fairwaves.co