Re: unsetenv() patch for TZ

[Corinna Vinschen <vinschen at redhat dot com>]

On Mar 30 11:38, Craig Howland wrote:
> On 06/07/2013 06:23 AM, Corinna Vinschen wrote:
> >Hi Craig,
> >
> >On Jun  6 17:33, Craig Howland wrote:
> >>On 07/09/2011 08:19 AM, Corinna Vinschen wrote:
> >>>Having said that, shouldn't the tzset() call better be removed in
> >>>_setenv_r and called in the affected functions?
> >>>
> [...]
> >Tzset alone is bigger than setenv/getenv combined, so by adding a tzset
> >call you raise the size of an executable using the environment functions
> >by more than 100%.
> setenv() already has the tzset() call.  Adding it to unsetenv(), also, to
> fix the latent bug will make no size difference to anyone's build other than
> the patch itself, as you would not be using unsetenv() without having used
> setenv().

That was the sole reason of discussing it.  I proposed to remove it from
setenv/unsetenv for the very reason that every usage of setenv/unsetenv
pulls in tzset, even for applications not using time functions at all.
Therefore calling tzset from setenv/unsetenv appears to me as a space
consuming shortcut.  I'm not concerned for Cygwin, obviously, but small
targets might profit from removing these calls.

>      OK.  While I disagree that this is the best approach, it is your
> prerogative and attached is a patch for the
> ctime()/localtime()/mktime()/strftime() approach.  It does have the benefit
> of also fixing the problem identified by Yaakov in "[PATCH] strftime: use
> tzname if TM_ZONE is NULL"
> (https://sourceware.org/ml/newlib/2015/msg00321.html).
>      Since ctime() calls localtime(), it is not directly patched (i.e. the
> patch only directly adds to 3 of the 4 functions mentioned).  The strftime.c
> patch also gets the wcsftime() function due to the shared source.
>      I made a minor size/efficiency call in strftime(), and put in two
> tzset() calls, one for %z and one for %Z, so that tzset() will only be
> called if it is needed, at the expense of one more function call in the
> overall size.  (Which is what Corinna says GLIBC does.0
>                 Craig

Thanks.  The setenv change is ok, but I have a bit of a problem with the
implemention in the time functions.  Apart from the strftime 'z' case,
all calls to tzset are outside the TZ_LOCK/TZ_UNLOCK bracket, but tzset
calls TZ_LOCK/TZ_UNLOCK, too.  As far as I can see it would be better to
implement _tzset_unlocked/_tzset_unlocked_r and call these from inside
the TZ_LOCK/TZ_UNLOCK bracket, wouldn't it?

Btw., was there some (historical?) reason that _setenv_r called tzset,
and not _tzset_r?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: pgpL9eiWI6d9o.pgp
Description: PGP signature