Hi, When USG_COMPAT is set, in localtime.c a long timezone (and int daylight) is declared. In strftime.c, also timezone is used, but no reference to 'extern long timezone' is define, neither do any of the provided header files. On most *NIX platforms, a system 'timezone' variable exists (see e.g. man tzset). My concerns are: a. The provided variable in localtime.c may conflict with the system one. b. The used variable in strftime.c may not related to the variable in localtime.c I think that compilers/linkers may complain about duplicate 'timezone' variables, or probably silently map the local timezone variable to the same one. Question: Is behavior intended? Or do I miss something? Regards, Kees
Date: Mon, 31 Aug 2015 11:11:15 +0000 From: Kees Dekker <Kees.Dekker@infor.com> Message-ID: <858F859BB4F2824EBAB5D4ED58214CB7011DEBF87B@NLBAWEXMBX3.infor.com> | Question: Is behavior intended? Or do I miss something? Technically I think you may be right - but the general rule for C (at least on unix like systems) is that uninitialised global variables are handled like fortran common blocks (if that means anything) - which means they can be declared as many times as needed, they all refer to the same object (with the same name of course) and there's no one single defining reference (though there can be, if one is initialised). C doesn't (or didn't in the early days) guarantee that - it depended on the system's linker, but unix ld works that way. So ... a. The provided variable in localtime.c may conflict with the system one. It won't. Of course, other things might conflict if you somehow get the system's localtime and the one from the tzcode linked into the same binary. b. The used variable in strftime.c may not related to the variable in localtime.c It will be the same thing. kre
Kees Dekker wrote:
My concerns are:
a. The provided variable in localtime.c may conflict with the system one.
Their types shouldn't conflict, as the type is standard. If the types actually do conflict it's a bug, and we should fix it, but I don't know of any such conflicts. In most production libraries the linkages of the variables do not conflict either, because the library definitions are weak. Even with old-fashioned libraries that do not use weak symbols (are there are any? you can't build a standard C system without them...), so long as localtime.c defines all the functions and variables that the corresponding library modules define we're fine, because the corresponding library modules will not be pulled in and our functions and variables will be used everywhere, as they should be.
b. The used variable in strftime.c may not related to the variable in localtime.c
Question: Is behavior intended? Or do I miss something?
The behavior is intended, in the sense that localtime.c is intended to be a complete implementation of the Unix library's time-related functions (other than system calls) and variables, and is intended to replace the Unix library's modules when linked into a C program.
I think that compilers/linkers may complain about duplicate 'timezone' variables, or probably silently map the local timezone variable to the same one.
The latter is intended. In practice the former doesn't seem to happen for reasons mentioned above. That being said, I can see one weird situation where it'd be a win to remove those variables' initializations. If you're in a system that uses the "Common" linkage model (see the C standard rationale), and if localtime.c says "long timezone;" and a system library module also says "long timezone;", and if the system library module defines some function F that we don't, and if F is used by the program, and if the system library module doesn't use weak symbols, then things will still work. But if localtime.c says "long timezone = 0;" the linker will complain about multiple definitions. So under this hypothetical environment (which we've never seen and aren't likely to), the attached proposed patch would be beneficial. Also, the attached patch saves a few words in traditional object modules, so it's a minor win for that reason anyway.
Hi Paul, Thanks for clarification. But why not then add an 'extern long timezone' line in either both localtime.c and strftime.c or in a header file of the olson code? That will probably better express that the variable 'timezone' is not of the tzcode library, but is assumed to be a global symbol, somewhere in the system libraries. I can imagine a bad scenario, i.e. mixing tzcode and system libraries, where tzcode (in localtime.c) adjusts the timezone variable, where after the end user uses system functions that use the same variable. In any case, this can be considered as bad programming if it is related to own source code, but it is out of the scope of control of programmers if the same happens by system functions that use implicitly the timezone variable under the hood. It would probably a clearer rule to say: either use olson tzcode, or use system libraries, and they both can be mixed, but should not affect each other. I.e. then the timezone variable should be renamed to a unique name, e.g. olson_timezone, and both localtime.c and strftime.c should use them. The disadvantage is that some Linux variants have to map the timezone variable to olson_timezone, because they you the tzcode variant in their system libraries. But that can be addressed by Linux engineers that rename/define #define olson_timezone timezone :-). If I understand the tzcode well, the timezone variable only affects strftime() when called with '%z' format speficier. So the effect is probably quite limited. Regards, Kees -----Original Message----- From: Paul Eggert [mailto:eggert@cs.ucla.edu] Sent: Monday, August 31, 2015 15:46 To: Kees Dekker; tz@iana.org Subject: Re: [tz] question regarding 'timezone' variable Kees Dekker wrote:
My concerns are:
a. The provided variable in localtime.c may conflict with the system one.
Their types shouldn't conflict, as the type is standard. If the types actually do conflict it's a bug, and we should fix it, but I don't know of any such conflicts. In most production libraries the linkages of the variables do not conflict either, because the library definitions are weak. Even with old-fashioned libraries that do not use weak symbols (are there are any? you can't build a standard C system without them...), so long as localtime.c defines all the functions and variables that the corresponding library modules define we're fine, because the corresponding library modules will not be pulled in and our functions and variables will be used everywhere, as they should be.
b. The used variable in strftime.c may not related to the variable in localtime.c
Question: Is behavior intended? Or do I miss something?
The behavior is intended, in the sense that localtime.c is intended to be a complete implementation of the Unix library's time-related functions (other than system calls) and variables, and is intended to replace the Unix library's modules when linked into a C program.
I think that compilers/linkers may complain about duplicate 'timezone' variables, or probably silently map the local timezone variable to the same one.
The latter is intended. In practice the former doesn't seem to happen for reasons mentioned above. That being said, I can see one weird situation where it'd be a win to remove those variables' initializations. If you're in a system that uses the "Common" linkage model (see the C standard rationale), and if localtime.c says "long timezone;" and a system library module also says "long timezone;", and if the system library module defines some function F that we don't, and if F is used by the program, and if the system library module doesn't use weak symbols, then things will still work. But if localtime.c says "long timezone = 0;" the linker will complain about multiple definitions. So under this hypothetical environment (which we've never seen and aren't likely to), the attached proposed patch would be beneficial. Also, the attached patch saves a few words in traditional object modules, so it's a minor win for that reason anyway.
Kees Dekker wrote:
But why not then add an 'extern long timezone' line in either both localtime.c and strftime.c or in a header file of the olson code? That will probably better express that the variable 'timezone' is not of the tzcode library, but is assumed to be a global symbol, somewhere in the system libraries.
That's not the assumption. localtime.c can be used standalone, e.g., even if the system library has no localtime function or associated variables. So we can't declare those variables in the way you suggest. Since this is only a theoretical problem, not a real one, I'm not inclined to worry about it all that much.
If I understand the tzcode well, the timezone variable only affects strftime() when called with '%z' format speficier. So the effect is probably quite limited.
The timezone variable is not used at all on platforms that define TM_GMTOFF. Platforms that do not define TM_GMTOFF are deficient anyway; for example, a call to localtime_r in one thread can mess up the value of 'timezone' in another. Really, anybody building a new platform should treat 'timezone' as a relic from the Dark Ages and not worry about it too much.
Hi, I forgot to tell the trigger why I initiated this email chain: I was busy with a trial migration to Visual Studio 2015. Visual Studio 2015 has renamed timezone to _timezone (note the leading underscore). As a result, both strftime.c and localtime.c do not compile anymore. So the problem is not theoretical. And the behavior of Microsoft/Visual Studio to add a leading underscore to variables/functions for ANSI variants, is unpleasant, but quite common... If tzcode would use a specialized variable: a. in the header file extern long olson_timezone b. in localtime.c: long timezone definition c. in strftime.c using olson_timezone d. and finaly: #if USG_COMPAT #define olson_timezone timezone #endif In my case, I then can use for Windows: #define olson_timezone _timezone Which solves my issue. Of course, I can decide to not use USG_COMPAT, which prevents me in using a relic from the dark ages :-). Not that dark IMO, since man tzset on Linux still shows the existence of timezone variable. So it is still used (and not all code is re-entrant/thread-safe). Since the migration was a trial (a final migration will probably planned later this year), I 'm not in hurry. Regards, Kees -----Original Message----- From: Paul Eggert [mailto:eggert@cs.ucla.edu] Sent: Monday, August 31, 2015 17:12 To: Kees Dekker; tz@iana.org Subject: Re: [tz] question regarding 'timezone' variable Kees Dekker wrote:
But why not then add an 'extern long timezone' line in either both localtime.c and strftime.c or in a header file of the olson code? That will probably better express that the variable 'timezone' is not of the tzcode library, but is assumed to be a global symbol, somewhere in the system libraries.
That's not the assumption. localtime.c can be used standalone, e.g., even if the system library has no localtime function or associated variables. So we can't declare those variables in the way you suggest. Since this is only a theoretical problem, not a real one, I'm not inclined to worry about it all that much.
If I understand the tzcode well, the timezone variable only affects strftime() when called with '%z' format speficier. So the effect is probably quite limited.
The timezone variable is not used at all on platforms that define TM_GMTOFF. Platforms that do not define TM_GMTOFF are deficient anyway; for example, a call to localtime_r in one thread can mess up the value of 'timezone' in another. Really, anybody building a new platform should treat 'timezone' as a relic from the Dark Ages and not worry about it too much.
Kees Dekker wrote:
Visual Studio 2015 has renamed timezone to _timezone (note the leading underscore). As a result, both strftime.c and localtime.c do not compile anymore.
Why doesn't localtime.c compile? The variables have diffent names and so should not interfere with each other. What's the error message? Do you still get an error with the patch I proposed and have installed in the experimental version on Github?
Hi, I rolled back by VS 2015 changes, so it is not very easy to get the compile error. I need to reapply/redo the VS2015 changes to reproduce the problem. If I have time tomorrow to do so, I will try... It was solved by replacing timezone by _timezone. I figured out this difference by comparing the Visual Studio 2013 and 2015 header files where timezone was defined. I was actually compiling my own localtime.c, which is a copy of a much older (I guess February 2013) variant of tzcode's locatime.c. Because own customizations were (unfortunately) made in the past, it is not very easy to test the latest github variant. I consider to turn off USG_COMPAT define, which is IMO pretty weird to have enabled on Windows. That will also solve my issue. Regards, Kees -----Original Message----- From: Paul Eggert [mailto:eggert@cs.ucla.edu] Sent: Tuesday, September 01, 2015 08:29 To: Kees Dekker; tz@iana.org Subject: Re: [tz] question regarding 'timezone' variable Kees Dekker wrote:
Visual Studio 2015 has renamed timezone to _timezone (note the leading underscore). As a result, both strftime.c and localtime.c do not compile anymore.
Why doesn't localtime.c compile? The variables have diffent names and so should not interfere with each other. What's the error message? Do you still get an error with the patch I proposed and have installed in the experimental version on Github?
On Sep 1, 11:24am, random832@fastmail.us (random832@fastmail.us) wrote: -- Subject: Re: [tz] question regarding 'timezone' variable | On Tue, Sep 1, 2015, at 10:47, Kees Dekker wrote: | | > I consider to turn off USG_COMPAT define, which is IMO pretty weird to | > have enabled on Windows. | | Since windows doesn't have tm_gmtoff in struct tm, make sure to test | that strftime works correctly with %z. The NetBSD _z changes added strftime_z and strftime_lz. Shouldn't those be adopted too? If not, why? christos
On 09/01/2015 09:45 AM, Christos Zoulas wrote:
The NetBSD _z changes added strftime_z and strftime_lz. Shouldn't those be adopted too? If not, why?
They're not needed on systems that have tm_gmtoff and tm_zone, as strftime can and should consult those struct tm members instead of consulting any separate timezone_t type. Systems that lack tm_gmtoff and tm_zone can't possibly be thread-safe in the presence of multiple time zones anyway, so we don't need to worry about adding strftime_z and strftime_lz to such systems.
On Sep 1, 10:40am, eggert@cs.ucla.edu (Paul Eggert) wrote: -- Subject: Re: [tz] question regarding 'timezone' variable | On 09/01/2015 09:45 AM, Christos Zoulas wrote: | > The NetBSD _z changes added strftime_z and strftime_lz. Shouldn't those be | > adopted too? If not, why? | | They're not needed on systems that have tm_gmtoff and tm_zone, as | strftime can and should consult those struct tm members instead of | consulting any separate timezone_t type. | | Systems that lack tm_gmtoff and tm_zone can't possibly be thread-safe in | the presence of multiple time zones anyway, so we don't need to worry | about adding strftime_z and strftime_lz to such systems. ... pt = _add(tzname[t->tm_isdst != 0], ... How do you access tzname which is a global making sure that it does not change under you by another thread? christos
Christos Zoulas wrote:
pt = _add(tzname[t->tm_isdst != 0], ...
How do you access tzname which is a global making sure that it does not change under you by another thread?
You don't. The code involving tzname isn't used on a system that has a working tm_zone member. Any system desiring thread-safety should have a struct tm with both tm_zone and tm_gmtoff members, as NetBSD does.
On Sep 1, 11:02pm, eggert@cs.ucla.edu (Paul Eggert) wrote: -- Subject: Re: [tz] question regarding 'timezone' variable | Christos Zoulas wrote: | > pt = _add(tzname[t->tm_isdst != 0], | > ... | > | > How do you access tzname which is a global making sure that it does not | > change under you by another thread? | | You don't. The code involving tzname isn't used on a system that has a working | tm_zone member. Any system desiring thread-safety should have a struct tm with | both tm_zone and tm_gmtoff members, as NetBSD does. Okay, but we should fix the NULL cases in strptime() then so that TM_ZONE is never NULL? christos #ifdef TM_ZONE if (t->TM_ZONE != NULL) pt = _add(t->TM_ZONE, pt, ptlim); else #endif /* defined TM_ZONE */ if (t->tm_isdst >= 0) pt = _add(tzname[t->tm_isdst != 0], pt, ptlim);
Christos Zoulas wrote:
Okay, but we should fix the NULL cases in strptime() then so that TM_ZONE is never NULL?
I assume you mean strftime, not strptime, as there's no strptime in tzcode. I guess the strftime code was written to be portable to localtime implementations that have tm_zone but set it to NULL, and also have tzname and set it correctly. I'm pretty sure there are no such implementations; certainly tzcode itself doesn't do that. The idea of tm_zone and tm_gmtoff implementations not knowing the time zone is problematic; even if tm_zone == NULL would denote that, what value would tm_gmtoff be set to? Also, since release 2014g, zdump has not worked with such implementations but nobody has reported problems with it. So the attached proposed patch removes this unnecessary complication, making strftime.c behave as zdump.c already does in this area.
On Sep 2, 7:47am, eggert@cs.ucla.edu (Paul Eggert) wrote: -- Subject: Re: [tz] question regarding 'timezone' variable | I assume you mean strftime, not strptime, as there's no strptime in tzcode. I was looking at the strptime implementation on NetBSD and forgot that this was not part of tzcode. I will see how to fix that code not to set TM_ZONE to NULL in some cases. | I guess the strftime code was written to be portable to localtime | implementations that have tm_zone but set it to NULL, and also have tzname and | set it correctly. I'm pretty sure there are no such implementations; certainly | tzcode itself doesn't do that. The idea of tm_zone and tm_gmtoff | implementations not knowing the time zone is problematic; even if tm_zone == | NULL would denote that, what value would tm_gmtoff be set to? Also, since | release 2014g, zdump has not worked with such implementations but nobody has | reported problems with it. So the attached proposed patch removes this | unnecessary complication, making strftime.c behave as zdump.c already does in | this area. Right, thanks I like this patch... christos
participants (5)
-
christos＠zoulas.com -
Kees Dekker -
Paul Eggert -
random832＠fastmail.us -
Robert Elz