[erlang-patches] Fix calendar:local_time_to_universal_time_dst/2 bug of return value -1 from mktime

[Björn Gustavsson]

2010/2/11 Kenji Rikitake <kenji.rikitake@REDACTED>:
> I would like the mktime fix of erl_time_sup.c to be included ASAP.

I will include a fix in a new branch in pu and unless it causes any
problems in our daily builds (unlikely), it will be included in R13B04.
See the end of the email for the fix and my description of it.

>> The documentation says that local_time_to_universal_time_dst/1
>> returns an empty list for a time that does not exist. We don't want
>> the same return value for a valid time that cannot be handled because
>> of an implementation detail.
>
> I will not argue against this decision for the time being.

We may change our minds if the patch is to be included in
a major release (such as R14, where some incompatibilities
are allowed) and the patch is ready in good time before the
release so we don't have to rush the review. I agree that calendar
will need some sort of update to properly handle badargs from
local_time_to_universal_time().

Here is my suggested patch (which will soon be included in pu):

Subject: [PATCH] erl_time_sup.c: test for error return from mktime()

When converting from local time to universal time in
erlang:localtime_to_universal_time/2, the return value
from mktime() is not checked, so if it returns -1
(an error return), the BIF will return a time just before
the start of the epoch (namely {{1969,12,31},{23,59,59}}).

There is a range check in erlang:localtime_to_universal_time/2
that causes an badarg exception if the arguments are out
their valid ranges (for example, year less than 1970 or
month greater than 12).

At least on FreeBSD systems, however, mktime() returns -1 when
the TZ is set to UTC and is_dst is set to true. Therefore
it is necessary to check the return value of mktime() and
cause a badarg exception if it is -1.

Reported by Kenji Rikitake and Paul Guyot. See:

  http://www.erlang.org/pipermail/erlang-bugs/2008-November/001077.html
---
 erts/emulator/beam/erl_time_sup.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/erts/emulator/beam/erl_time_sup.c
b/erts/emulator/beam/erl_time_sup.c
index c15f85f..51202fb 100644
--- a/erts/emulator/beam/erl_time_sup.c
+++ b/erts/emulator/beam/erl_time_sup.c
@@ -650,6 +650,9 @@ local_to_univ(Sint *year, Sint *month, Sint *day,
     t.tm_sec = *second;
     t.tm_isdst = isdst;
     the_clock = mktime(&t);
+    if (the_clock == -1) {
+	return 0;
+    }
 #ifdef HAVE_GMTIME_R
     gmtime_r(&the_clock, (tm = &tmbuf));
 #else


-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB