benklop / microblog-purple

Automatically exported from code.google.com/p/microblog-purple
GNU General Public License v3.0
0 stars 0 forks source link

Incorrect timestamps #183

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I'm getting incorrect timestamps (one hour off). I noticed this since DST 
ended here (last week), although I can't swear it wasn't already happening 
before that.

So I poked around the relevant code a bit, not really expecting to be able 
to fix it, and it turns out the timezone handling is... somewhat confused. 
I cleaned it up a bit; not sure if it affects other protocols, since I only 
use twitter, but at least twitter works correct for me with this patch.

<TECHNICAL>
There are three timezones involved: the timezone in the XML from the server 
(always +0000 in the case of twitter.com), the localtime when the message 
was sent, and the current localtime (which are usually the same, unless you 
crossed a DST boundary, but that's a corner case that isn't too important).

So problem #1 was that mb_mktime() was expecting to apply the current 
localtime's DST flag to the server timestamp; the results of that are 
potentially nonsensical.

Problem #2 was using mktime() in the end to convert a struct tm which is in 
some arbitrary timezone (GMT in the case of twitter.com), while actually 
mktime() expects a localtime tm. The results can be nondeterministic. You 
were later offseting this by adding the local timezone offset on twitter.c, 
but since that's the *current* offset, that can again pose a problem, 
depending on the message's age. (Not to mention it's technically incorrect, 
but I may be OCD'ing here. The point being that mb_mktime() returns an 
integer that isn't correct either as local or GMT time...)

I used timegm() instead; the manpage says it's supposedly not portable, but 
AFAICS it's supported by all OSes that matter; if someone has a system 
without timegm(), it would be simple to add a naive implementation, as 
described by the manpage.

Problem #3 which I *think* was the actual culprit was not initializing the 
tm_isdst flag in the struct tm. I initialized it to 0, although it's made 
irrelevant by using timegm() which ignores that flag.
</TECHNICAL>

Either way, it works for me with this :-P

Original issue reported on code.google.com by lalo.martins on 3 Mar 2010 at 1:57

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the patch. I tested it on Ubuntu and it works well. Patch landed in 
SVN.

Original comment by somsaks on 12 Mar 2010 at 10:57