Re: unsetenv() patch for TZ

[Craig Howland <howland at LGSInnovations dot com>]

On 07/09/2011 08:19 AM, Corinna Vinschen wrote:
> Re: unsetenv() patch for TZ
>
> On Jul  8 14:24, Howland Craig D (Craig) wrote:
> > The current implementation of _setenv_r() looks to see if the
> > environment
> > variable being set (or changed) is TZ, calling tzset() when it is so
> > that
> > the timezone is automatically kept matching the environment without user
> > intervention.
> >
> > However, _unsetenv_r() does not provide the same feature, likely leading
> > to a problem when TZ is unset.
> >
> > POSIX says nothing about this feature under setenv() and unsetenv(),
> > so it would be possible to make the behavior consistent by either adding
> > to unset or removing from set.  However, adding to unset seems far less
> > likely to cause backwards-compatibility problems than removing from set.
> >
> > The attached patch addresses the issue by adding a tzset() call to
> > _unsetenv_r() when TZ is unset.
>
> Thanks for the patch, but I'm wondering if calling tzset() in _setenv_r
> is the right thing to do.  I had a look into SUSv4 and it appears that
> the only functions which are supposed to change the timezone information
> are ctime, localtime, mktime, strftime, and an explicit call to tzset by
> the application.
>
> "Appears", because I couldn't find any definitive answer.
>
> Having said that, shouldn't the tzset() call better be removed in
> _setenv_r and called in the affected functions?
>
> Corinna
Corinna:

It's been a while, but to return to this since I'm running into it again and 
would like to get away from a local workaround:

Good question.  But, no, I suggest that it is better to make the addition to 
unsetenv() rather than to use your suggestion for footprint reasons (in addition 
to my original less-likely-to-cause-backwards-compatibility reason).

If we were to remove tzset() from setenv and add it to ctime(), localtime(), 
mktime(), and strftime(), we would be forcing the environment functions to be 
linked when any of those 4 functions were called.  (They are not now.  In 
libc/time, only tzset() currently touches an environment function.  A simple 
grep shows env in strftime, but that is in the regression test driver.)  I would 
guess that many small applications that want to deal with time do not actually 
use environment variables at all.  (I certainly do.) Having tzset() called by 
unsetenv() and setenv() can equally be claimed to be pulling tzset() in when 
perhaps it might not be wanted, but this seems a more likely case than the 
former.  That is, if you are using environment variables, it seems much more 
likely that you'd also be using time functions at the same time than the flip case.

Addressing your SUSv4 comment, I looked at POSIX.1-2013, and partially agree 
with your "apparent" comment, although I think that they may have updated with 
respect to 2008 it to make it a little more clear.

Firstly, in the definition of TZ:  "The contents of the environment variable 
named TZ shall be used by the ctime(), ctime_r(), localtime(), localtime_r() 
strftime(), mktime(), functions, and by various utilities, to override the 
default timezone."  Note the "and by various functions" part, which expands your 
list to an open-ended one.  While this perhaps makes it sound like maybe tzset() 
should be called by these, the base requirement is TZ contents, which only 
implies tzset().

Next, from the definition of mktime(), "Local timezone information shall be set 
as though mktime() called tzset()."  Note specifically the "as though," which 
seems to be clearly allowing it to not be directly called.  Having it embedded 
in setenv() and unsetenv() would give the same apparent operation and therefore 
meet the requirement.

Additionally, the environment is not required to be thread-safe, so there is not 
any kind of timing reason to prefer the use in the environment functions versus 
the time functions.  For example, if one thread were calling setenv() to change 
TZ, it would need to ensure that the setenv() call were complete before a 
different thread could call mktime(), regardless of which function calls tzset().

In summary, I think that technically either calling tzset() from the related 
time functions or from the environment functions could work properly and be 
standard-compliant.  But I think that the environment method better fits 
newlib's desire for small footprints, and suggest that route.

If you agree, you could apply the patch from 
http://sourceware.org/ml/newlib/2011/msg00297.html (which is still valid), but 
if the environment method is a must, I'll perhaps work on that in the next week 
or two.

Craig