Ticket #391 (Fixed)Thu Jul 17 21:53:26 UTC 2014
localtime() ignores manual timezone configuration
| Reported by: | Rick Murray (539) | Severity: | Normal |
| Part: | RISC OS: Software library | Release: | |
| Milestone: | Status | Fixed |
Details by Rick Murray (539):
The localtime() function determines the time zone offset by a bizarre method of reading the DST flag and then calling Territory_ReadTimeZones. Unfortunately, in territories with multiple timezones (such as the USA), this returns the first timezone defined. If you are using a different one (say, PST instead of EST), there is no way that your machine will ever return the correct local time.
Fix? I presume the complicated code is to support the enhancements to the setlocale() function where you can specify custom time zones on a per-application basis. Fair enough.
However when the standard default locale is in use, just call Territory_ReadCurrentTimeZone and be done with it.
More info here: https://www.riscosopen.org/forum/forums/4/topic…
Changelog:
Modified by Rick Murray (539) Sun, July 20 2014 - 14:33:22 GMT
- Attachment added: time.txt
Fix is as follows:
<pre>struct tm *localtime(const time_t *timer)
{
time_t t;
static struct tm _tms;
int dst;
int territory;
int v;
_kernel_swi_regs r;
}</pre>
Tested with UK: GMT, BST, CET, CEST (my version of UK that includes CET/CEST).
Tested with USA: EST, EDT, PST, AKST.
In all cases, the time returned by asctime(localtime(&meh)) matches *Time.
In case the bug tracker mangles the code, I have attached the “time” source.
Modified by Sprow (202) Sat, August 02 2014 - 08:39:44 GMT
- Summary changed from localtime() is broken to localtime() ignores manual timezone configuration
- Part changed from Unspecified to RISC OS: Software library
Awaits regression testing per
https://www.riscosopen.org/forum/forums/4/topic…
Suggest old and new C library on 3.10 3.50 3.60 3.70 4.0x with whatever the ROM vintage territory manager and UK territory is. Can stop at 4.0x since 5.0x is basically a 32 bit version of same.
Modified by Rick Murray (539) Mon, February 16 2015 - 20:46:05 GMT
Followed up here: https://www.riscosopen.org/forum/forums/4/topic…
RISC OS 3.10 (Archie emulator) and RISC OS 3.5 (RedSquirrel) both appear to exhibit a small time disparity of 7 minutes and 13 seconds. No obvious cause for this has been identified, and RISC OS 3.7 on the same RedSquirrel does NOT misbehave. Neither does RISC OS 5.
My suggestion would be a #ifdef to provide the existing code to older clients using the PlingBoot version of CLib (as the problem was never noticed on them before due to the lack of softloaded territory modules, etc etc) while providing the corrected code to RISC OS 5 (so “going forward” it can work correctly).
Modified by Sprow (202) Thu, February 19 2015 - 23:06:34 GMT
Little puzzled at what’s being compared in the posted results, why compare a 32 bit neutral CLib with your changed one?
We want to compare the
ROM C library + ROM territory + ROM territory module
with a test application (compiled for APCS-R) against
Updated C library + ROM territory + ROM territory module
to see if the case of the default C locale isn’t affected by the update. I wouldn’t expect the result of the non default locale to be right prior to RISC OS 5 since the territory SWIs didn’t return anything useful for anything but the base territory they represent, so as long as it doesn’t data abort or anything silly it’d be OK to get a junk result there.
Rinse & repeat for each OS version.
There’s no need for #ifdef’s as there aren’t any new SWIs implicated here.
Modified by Rick Murray (539) Wed, March 18 2015 - 20:31:40 GMT
Comparing a 32 bit neutral CLib as that is what people will be using on the systems affected. You could probably count people using ROM CLib on the fingers of one hand…
“to see if the case of the default C locale isn’t affected by the update” – how do you mean exactly? I was looking at the result of calling gmtime() and localtime(). Is there something else I ought to be testing?
The #ifdef suggestion was because the same updated CLib appears to give different results for localtime() on RISC OS 3.5 and RISC OS 3.7 (the 433 seconds quirk). As you point out, changing territory and timezone was a bit of a non-starter on older systems, so having an #ifdef for building 32 bit (RISC OS 5) versions of the library would mean that the issue is corrected for new machines, and older ones receive the same (broken) code as before, which was not noticed in, oh, twenty-odd years. ;-)
Modified by Sprow (202) Thu, March 19 2015 - 09:19:21 GMT
“Comparing a 32 bit neutral CLib as that is what people will be using on the systems affected”, my point being that you said you tested the existing neutral C lib (which this ticket proposes is where there is a bug) with your fixed neutral C lib, which is 2 tests yielding only 1 result. Test ROM C lib and with fixed neutral C lib is 2 tests yielding 2 results.
“to see if the case of the default C locale isn’t affected by the update”, a simple round trip convert and convert back with gmtime() localtime() sounds fine.
Modified by Rick Murray (539) Sat, May 07 2016 - 13:13:14 GMT
Test results here: https://www.riscosopen.org/forum/forums/4/topic…
Modified by Jeffrey Lee (213) Sun, May 22 2016 - 12:46:57 GMT
- Status changed from Open to Fixed
These fixes should now be available in CLib 5.90
Modified by Rick Murray (539) Fri, June 10 2016 - 17:45:06 GMT
Tested with recent RISC OS build and a standalone UK territory that has CET and CEST timezones added.
Fix confirmed.
