Hi, On Fri, 24 Jun 2011 13:02:35 Sebastian Harl wrote: > On Fri, Jun 24, 2011 at 12:28:22PM +0200, Bruno Prémont wrote: > > On Fri, 24 Jun 2011 12:10:41 Aurelien ROUGEMONT wrote: > > > I'm working for a registrar. We use intensively isc-bind, and since a > > > few month collectd's bind plugin. > > > > > > We were experiencing a very specific behavior from this plugin only. The > > > datetime returned was shifted. After some diggin', i figured what was > > > the problem : the current code was not handling timesavings. > > > > > > the bind plugin works like this : > > > * query to xmlrpc bind webservice > > > * process data : bind statistics and datetime > > > * returns processed data > > > > > > Since it's the only plugin returning a datetime not using timesavings i > > > read [1] and wrote a quick fix that : > > > > > > 1- substracts the constant timezone to the "about to be returned datetime" > > > 2- adds to the result timezone and timesaving ( with *localtime() ) > > > 3- returns the result instead of the UTC datetime + timezone
The datetime returned by Bind is UTC but conversion to time_t via localtime() offsets by timezone AND daylight saving. > > > [1] http://www.gnu.org/s/hello/manual/libc/Time-Zone-Functions.html > > > > The more simple/safe solution should be to revert timegm() -> mktime() > > change done in > > http://git.verplant.org/?p=collectd.git;a=commitdiff;h=1641c82ec4e14968ea31021dfb8b520df5f4483a > > timegm() is a GNU extension, thus, using it should be avoided. So, imho, > the approach taken in the patch is fine. timegm() is also available on some of the BSDs. I would say it's better to use timegm() when available and fallback to some workaround when not available instead of just doing work-around. > > In your patch you refer to a "timezone" variable, where does that one > > come from? > > It's defined in 'time.h' -- it's defined as a mandatory XSI extension of > POSIX. Ok, it has a man-page too but there it's mentioned as: timezone: _SVID_SOURCE || _XOPEN_SOURCE which is not POSIX, just XOPEN_SOURCE (and _SVID_SOURCE also provides timegm()) Then timezone is no enough, daylight-saving must be accounted for as well (and that starts to get painfull). > > In addition, localtime() and friends depend on process-global timezone > > setting which is rather bad in a multithreaded process such as collectd! > > Hrm … do you really consider that to be an issue? Imho, having the > timezone a process-global setting is fine even in a multi-threaded > process. I'd rather consider code buggy that touches that setting ;-) As long as timezone is never changed it's fine (but who knows what some of the libraries used by plugins might do). Bruno _______________________________________________ collectd mailing list [email protected] http://mailman.verplant.org/listinfo/collectd
