time zone problems with util/timegm.c on mingw32

[Fabian Keil]

David Shaw <dshaw at jabberwocky.com> wrote:

> On Sun, Jun 10, 2007 at 07:00:22PM +0200, Fabian Keil wrote:
> > A while ago I needed a fallback timegm() version
> > for Privoxy and chose the one from gnupg 1.x's
> > util/timegm.c because it appeared to be working
> > on mingw32 and the license was compatible.
> > 
> > Apparently I didn't test it properly, because I
> > just noticed that it causes time zone problems
> > if getenv("TZ") is NULL. On my Windows test system
> > this (now) seems to be the case.
> > 
> > As a result the first timegm() call changes the time
> > zone settings (for me it causes a 2h offset for
> > localtime() called later on) while the next timegm()
> > call fixes the problem again. The timegm() call after
> > that restarts the cycle ...
> > 
> > This, and a minor memory leak, can be fixed
> > (on my mingw32 test system) with:
> > 
> > --- util/timegm.c       Wed Jul 27 19:02:56 2005
> > +++ util/timegm.c.patched       Sun Jun 10 18:52:30 2007
> > @@ -54,13 +54,14 @@
> >           strcpy(old_zone,"TZ=");
> >           strcat(old_zone,zone);
> >           putenv(old_zone);     
> > +         free(old_zone);
> 
> That's intentional, and not a leak.  putenv() is supposed to insert
> its argument directly into the environment.  If you free that
> argument, you corrupt the environment when (eventually) something else
> uses that memory.  There are some nonstandard putenv()s that make a
> copy like setenv() does, but the trick is knowing which you have - it
> is safest to assume that your putenv() follows the standard.
> 
> >  #ifdef HAVE_UNSETENV
> >      unsetenv("TZ");
> >  #else
> > -    putenv("TZ");
> > +    putenv("TZ=");
> >  #endif
> 
> This is mingw-specific (and wrong, as "TZ=" should insert an empty
> string, and not remove TZ from the environment).  Most platforms use
> "TZ" (without the = sign) to remove something from the environment.

I guess I'll better #ifdef both cases for mingw32 then.

> Even that, however, is non-standard behavior, which is why we try to
> use unsetenv() when possible.  Arguably for the most standards-based
> code in GnuPG (though probably not your usage), if unsetenv isn't
> available, we should just give up and leak the memory.

I think the main point of unsetting TZ there is to make sure
timegm() calls don't have side effects for other time functions
called later on.

Without the last putenv() call to unset TZ, timegm() would
permanently set the time zone to UTC if TZ wasn't already
set to something at the beginning (because the last tzset()
call wouldn't restore the original time zone).

A few leaked bytes per call are hard to notice, but if
localtime() gets an offset, things get interesting ...

Thanks a lot for your detailed response, David.

Fabian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : /pipermail/attachments/20070611/c60dec9d/attachment.pgp