This is the mail archive of the mailing list for the newlib project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: unsetenv() patch for TZ

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?



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 (which is still valid), but if the environment method is a must, I'll perhaps work on that in the next week or two.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]