| Author |
Message |
< Erlang bugs mailing list ~ [erlang-patches] Fix calendar:local_time_to_universal_time_d |
| jj1bdx |
 Posted: Wed Feb 10, 2010 11:20 pm |
 |
|
|
User
Joined: 11 Sep 2008
Posts: 92
|
Bjorn and all:
I would like the mktime fix of erl_time_sup.c to be included ASAP.
(check out
git fetch git://github.com/jj1bdx/otp.git local_to_univ
for the patch details)
In the message <6672d0161002100626y36ab72f4oc451c9478865be75@mail.gmail.com>
dated Wed, Feb 10, 2010 at 03:26:07PM +0100,
Bjorn Gustavsson <bgustavsson@gmail.com> writes:
> We don't want the change in calendar.erl.
>
> 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 consider using the patch for erl_time_sup.c, if you can
> provide some information to show that it is really needed. Note
> that the BIF does a range check of its arguments. Are there some
> values of the arguments that slip through the range check and
> cause mktime() to return -1?
>
> Since such a change is considered a bug fix, we can accept
> it after the feature freeze today.
Paul Guyot summarized the case in
http://www.erlang.org/pipermail/erlang-bugs/2008-November/001077.html
as:
(quote)
"I found a bug in Erlang/OTP R12B-4 that occurs on FreeBSD 7.0 with TZ
set to UTC. On such systems, mktime(3) returns -1 when is_dst is set
to true. According to SUSv3, the OS can return -1 when the time since
Epoch cannot be represented, and therefore FreeBSD is perfectly
allowed to do so.
http://www.opengroup.org/onlinepubs/009695399/functions/mktime.html
"
(unquote)
The POSIX doc for mktime in the above opengroup.org link shows the
following definition for the return value:
(quote)
RETURN VALUE
The mktime() function shall return the specified time since the
Epoch encoded as a value of type time_t. If the time since the Epoch
cannot be represented, the function shall return the value
(time_t)-1 and may set errno to indicate the error.
(unquote)
And a source code of error checking example as:
(quote)
/* What day of the week is July 4, 2001? */
#include <stdio.h>
#include <time.h>
struct tm time_str;
char daybuf[20];
int main(void)
{
time_str.tm_year = 2001 - 1900;
time_str.tm_mon = 7 - 1;
time_str.tm_mday = 4;
time_str.tm_hour = 0;
time_str.tm_min = 0;
time_str.tm_sec = 1;
time_str.tm_isdst = -1;
if (mktime(&time_str) == -1) /* <-- HERE */
(void)puts("-unknown-");
else {
(void)strftime(daybuf, sizeof(daybuf), "%A", &time_str);
(void)puts(daybuf);
}
return 0;
}
(unquote)
So I think the -1 return value of mktime should be properly handled.
Regards,
Kenji Rikitake
>
> --
> Björn Gustavsson, Erlang/OTP, Ericsson AB
>
> ________________________________________________________________
> erlang-patches (at) erlang.org mailing list.
> See http://www.erlang.org/faq.html
> To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
________________________________________________________________
erlang-bugs (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-bugs-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
 
|
| Guest |
| Posted: Thu Feb 11, 2010 6:39 am |
|
|
|
Guest
|
2010/2/11 Kenji Rikitake <kenji.rikitake@acm.org>:
> 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
________________________________________________________________
erlang-bugs (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-bugs-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| jj1bdx |
| Posted: Thu Feb 11, 2010 8:25 am |
|
|
|
User
Joined: 11 Sep 2008
Posts: 92
|
I'll keep this terse:
In the message <6672d0161002102238w7a62fa55gbf1d62758674b371@mail.gmail.com>
dated Thu, Feb 11, 2010 at 07:37:55AM +0100,
Björn Gustavsson <bgustavsson@gmail.com> writes:
> 2010/2/11 Kenji Rikitake <kenji.rikitake@acm.org>:
> > 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 patch is confirmed and I have no objection.
> >> 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().
I'd be glad if Erlang/OTP team will review the semantics of
local_time_to_universal_time() again before R14.
> Here is my suggested patch (which will soon be included in pu):
>
> Subject: [PATCH] erl_time_sup.c: test for error return from mktime()
Patch confirmed.
(rest deleted)
Regards,
Kenji Rikitake
________________________________________________________________
erlang-bugs (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-bugs-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
|
|
All times are GMT
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You cannot attach files in this forum
You cannot download files in this forum
|
|
|
Digg It
Del.icio.us
Reddit
Facebook
Stumble Upon
Technorati
