[collectd] Collectd bind plugin patch: timesaving problem

[Sebastian Harl]

Hi,

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

> > [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.

> 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.

> 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 ;-)

Cheers,
Sebastian

-- 
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://mailman.verplant.org/pipermail/collectd/attachments/20110624/644b92f2/attachment.pgp>