Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Useless day range check in Time::Local #745

Closed
p5pRT opened this issue Oct 20, 1999 · 10 comments
Closed

Useless day range check in Time::Local #745

p5pRT opened this issue Oct 20, 1999 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 20, 1999

Migrated from rt.perl.org#1668 (status was 'resolved')

Searchable as RT1668$

@p5pRT
Copy link
Author

p5pRT commented Oct 20, 1999

From allen@grumman.com

Can we please get rid of the useless day range check in Time​::Local?
Pleeease? I recall this being argued before.

  $ perl -MTime​::Local -e 'print scalar localtime timelocal 0,0,0,280,0,99'
  Day '280' out of range 1..31 at -e line 1

It is quite convenient to be able to use timelocal to convert from "julian"
YYDDD format to others.

The warning serves no purpose anyway, since the following is not caught

  $ perl -MTime​::Local -e 'print scalar localtime timelocal 0,0,0,31,1,99'
  Wed Mar 3 00​:00​:00 1999

Ie, there is no 31st day in February.

Appended is the trivial patch against _60 in case the pumpking
agrees with me.

John.

$ diff -c Local.pm.orig Local.pm
*** Local.pm.orig Tue Jul 20 13​:18​:01 1999
--- Local.pm Wed Sep 1 13​:28​:49 1999
***************
*** 74,80 ****
  $year = $_[5];
  $month = $_[4];
  croak "Month '$month' out of range 0..11" if $month > 11 || $month < 0;
- croak "Day '$_[3]' out of range 1..31" if $_[3] > 31 || $_[3] < 1;
  croak "Hour '$_[2]' out of range 0..23" if $_[2] > 23 || $_[2] < 0;
  croak "Minute '$_[1]' out of range 0..59" if $_[1] > 59 || $_[1] < 0;
  croak "Second '$_[0]' out of range 0..59" if $_[0] > 59 || $_[0] < 0;
--- 74,79 ----

@p5pRT
Copy link
Author

p5pRT commented Oct 20, 1999

From @gsar

On Wed, 20 Oct 1999 15​:00​:07 EDT, "John L. Allen" wrote​:

[ This is resend of my message of a month or more ago. Is there some reason
this cannot go in?

It went in, and then came back out.

  [ 4247] By​: gsar on 1999/09/28 17​:36​:59
  Log​: revert change#4115 (breaks libwww's base/date.t); could be
  reworked to enable it conditional on $Time​::Local​::nocroak
  or some such
  Branch​: perl
  ! lib/Time/Local.pm

Sarathy
gsar@​ActiveState.com

@p5pRT
Copy link
Author

p5pRT commented Oct 20, 1999

From [Unknown Contact. See original ticket]

On Wed, Oct 20, 1999 at 12​:37​:48PM -0700, Gurusamy Sarathy wrote​:

On Wed, 20 Oct 1999 15​:00​:07 EDT, "John L. Allen" wrote​:

[ This is resend of my message of a month or more ago. Is there some reason
this cannot go in?

It went in, and then came back out.

\[  4247\] By&#8203;: gsar                                  on 1999/09/28  17&#8203;:36&#8203;:59
    Log&#8203;: revert change\#4115 \(breaks libwww's base/date\.t\); could be 
     reworked to enable it conditional on $Time&#8203;::Local&#8203;::nocroak
     or some such
 Branch&#8203;: perl
       \! lib/Time/Local\.pm

Hmm. How about if we separate the range checking ability from the
time calculation but leave it on by default? That way people such as
myself can say something like​:

  use Time​::Local 'norange'; # or whatever

and just have timelocal() do date calculations and if we want range
checking, we can call the range_check() routine manually.

Comments?

-Scott
--
Jonathan Scott Duff
duff@​cbi.tamucc.edu

@p5pRT
Copy link
Author

p5pRT commented Oct 20, 1999

From [Unknown Contact. See original ticket]

On Wed, 20 Oct 1999, Gurusamy Sarathy wrote​:

On Wed, 20 Oct 1999 15​:00​:07 EDT, "John L. Allen" wrote​:

[ This is resend of my message of a month or more ago. Is there some reason
this cannot go in?

It went in, and then came back out.

\[  4247\] By&#8203;: gsar                                  on 1999/09/28  17&#8203;:36&#8203;:59
    Log&#8203;: revert change\#4115 \(breaks libwww's base/date\.t\); could be 
     reworked to enable it conditional on $Time&#8203;::Local&#8203;::nocroak
     or some such
 Branch&#8203;: perl
       \! lib/Time/Local\.pm

Ok, thanks. "No change, however simple, is without consequences."

I'll see about doing that Jonathan Scott Duff suggested.

John.

@p5pRT
Copy link
Author

p5pRT commented Oct 22, 1999

From [Unknown Contact. See original ticket]

On Wed, 20 Oct 1999 duff@​cbi.tamucc.edu wrote​:

On Wed, Oct 20, 1999 at 12​:37​:48PM -0700, Gurusamy Sarathy wrote​:

On Wed, 20 Oct 1999 15​:00​:07 EDT, "John L. Allen" wrote​:

[ This is resend of my message of a month or more ago. Is there some reason
this cannot go in?

It went in, and then came back out.

\[  4247\] By&#8203;: gsar                                  on 1999/09/28  17&#8203;:36&#8203;:59
    Log&#8203;: revert change\#4115 \(breaks libwww's base/date\.t\); could be 
     reworked to enable it conditional on $Time&#8203;::Local&#8203;::nocroak
     or some such
 Branch&#8203;: perl
       \! lib/Time/Local\.pm

Hmm. How about if we separate the range checking ability from the
time calculation but leave it on by default? That way people such as
myself can say something like​:

use Time&#8203;::Local 'norange';     \# or whatever

and just have timelocal() do date calculations and if we want range
checking, we can call the range_check() routine manually.

Comments?

I like it! As promised, below is a patch against _62. First, some notes
and questions​:

0. I didn't see much point to a range_check() sub, so I added none.

1. As a purely cosmetic part of the patch, I removed the line

  my($lday,$gday) = ($lt[7],$gt[7]);

  since $lday and $gday are never used.

2. I use an import() function that checks for certain "flags" passed in,
  sets package variables accordingly, and passes the remaining args,
  which presumably are exportable things, to Exporter. Is this considered
  bad style? I have done such a thing in in-house modules as well, and
  have even thought that Exporter should have this capability added.
  Any comments?

3. I use the flag 'no_range_check' to turn off the range checking. Too
  long?

4. I wanted to be able to mention in the pod that you could use large hours
  and minute values (as you can with days and seconds) and get consistent
  results. But there seems to be an off-by-one-hour problem​:

  use Time​::Local qw( no_range_check );
  print scalar localtime timelocal 0,60,0,1,0,99;
  # gives​: Fri Jan 1 00​:00​:00 1999
  print scalar localtime timelocal 0,120,0,1,0,99;
  # gives​: Fri Jan 1 01​:00​:00 1999
  print scalar localtime timelocal 0,0,24,1,0,99;
  # gives​: Fri Jan 1 23​:00​:00 1999
  print scalar localtime timelocal 0,0,25,1,0,99;
  # gives​: Sat Jan 2 00​:00​:00 1999

  Is this perhaps a Daylight Savings Time problem? Or is there something
  else going on. It _does_ work when using 'gmtime timegm'. I leave it
  to brighter bulbs than mine to ponder.

John
--

*** Local.pm.orig Thu Oct 14 14​:23​:22 1999
--- Local.pm Fri Oct 22 15​:37​:00 1999
***************
*** 3,10 ****
  require Exporter;
  use Carp;
 
! @​ISA = qw(Exporter);
! @​EXPORT = qw(timegm timelocal);
 
  # Set up constants
  $SEC = 1;
--- 3,21 ----
  require Exporter;
  use Carp;
 
! @​ISA = qw( Exporter );
! @​EXPORT = qw( timegm timelocal );
! @​EXPORT_OK = qw( $no_range_check );
!
! sub import {
! my $package = shift;
! my @​args;
! for (@​_) {
! $no_range_check = 1, next if $_ eq 'no_range_check';
! push @​args, $_;
! }
! Time​::Local->export_to_level(1, $package, @​args);
! }
 
  # Set up constants
  $SEC = 1;
***************
*** 51,57 ****
 
  my $tzsec = ($gt[1] - $lt[1]) * $MIN + ($gt[2] - $lt[2]) * $HR;
 
- my($lday,$gday) = ($lt[7],$gt[7]);
  if($lt[5] > $gt[5]) {
  $tzsec -= $DAY;
  }
--- 62,67 ----
***************
*** 73,83 ****
  sub cheat {
  $year = $_[5];
  $month = $_[4];
! croak "Month '$month' out of range 0..11" if $month > 11 || $month < 0;
! croak "Day '$_[3]' out of range 1..31" if $_[3] > 31 || $_[3] < 1;
! croak "Hour '$_[2]' out of range 0..23" if $_[2] > 23 || $_[2] < 0;
! croak "Minute '$_[1]' out of range 0..59" if $_[1] > 59 || $_[1] < 0;
! croak "Second '$_[0]' out of range 0..59" if $_[0] > 59 || $_[0] < 0;
  $guess = $^T;
  @​g = gmtime($guess);
  $lastguess = "";
--- 83,95 ----
  sub cheat {
  $year = $_[5];
  $month = $_[4];
! unless ($no_range_check) {
! croak "Month '$month' out of range 0..11" if $month > 11 || $month < 0;
! croak "Day '$_[3]' out of range 1..31" if $_[3] > 31 || $_[3] < 1;
! croak "Hour '$_[2]' out of range 0..23" if $_[2] > 23 || $_[2] < 0;
! croak "Minute '$_[1]' out of range 0..59" if $_[1] > 59 || $_[1] < 0;
! croak "Second '$_[0]' out of range 0..59" if $_[0] > 59 || $_[0] < 0;
! }
  $guess = $^T;
  @​g = gmtime($guess);
  $lastguess = "";
***************
*** 136,141 ****
--- 148,171 ----
  the values provided. While the day of the month is expected to be in
  the range 1..31, the month should be in the range 0..11.
  This is consistent with the values returned from localtime() and gmtime().
+
+ Also worth noting is that due to popular demand, we have added the
+ ability to disable the range checking that would normally occur on the
+ input $sec, $min, $hours, $mday, and $mon values. You can do this by
+ setting $Time​::Local​::no_range_check = 1, or by invoking the module
+ with use Time​::Local qw( no_range_check ). This enables you to abuse
+ the terminology somewhat and gain the flexibilty to do things like
+
+ use Time​::Local qw( no_range_check );
+ # The 365th day of 1999
+ print scalar localtime timelocal 0,0,0,365,0,99;
+ # The twenty thousandth day since 1970
+ print scalar localtime timelocal 0,0,0,20000,0,70;
+ # And even the 10,000,000th second since 1999!
+ print scalar localtime timelocal 10000000,0,0,1,0,99;
+
+ Your mileage may vary when trying this trick with minutes and hours,
+ and it doesn't work at all for months.
 
  Strictly speaking, the year should also be specified in a form consistent
  with localtime(), i.e. the offset from 1900.

@p5pRT
Copy link
Author

p5pRT commented Oct 22, 1999

From [Unknown Contact. See original ticket]

On Fri, Oct 22, 1999 at 04​:07​:31PM -0400, John L. Allen wrote​:

On Wed, 20 Oct 1999 duff@​cbi.tamucc.edu wrote​:

0. I didn't see much point to a range_check() sub, so I added none.

Do you honestly see no utility to having a routine range check your
seconds, minutes, days, months, years, etc. separate from the routine
that does time calculations? They are orthogonal concepts should they
not be coded so?

2. I use an import() function that checks for certain "flags" passed in,
sets package variables accordingly, and passes the remaining args,
which presumably are exportable things, to Exporter. Is this considered
bad style? I have done such a thing in in-house modules as well, and
have even thought that Exporter should have this capability added.
Any comments?

Me too.

BTW, in the long run I'd like to see a re-write of Time​::Local that​:
  a) is correct everywhere
  b) is fast
  c) returns undef on failure instead of -1
  d) is smart about variable timezones

If I had the tuits, I'd start hacking, but unfortunately I don't. I
also seem to recall long ago someone on p5p saying "I have a perfect
Time​::Local, unfortunately there isn't enough space in the margin of
my time to implement it" or something, so I'm hoping that if I didn't
imagine it, whoever that is will speak up.

-Scott
--
Jonathan Scott Duff
duff@​cbi.tamucc.edu

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 1999

From [Unknown Contact. See original ticket]

duff@​cbi.tamucc.edu wrote

BTW, in the long run I'd like to see a re-write of Time​::Local that​:
a) is correct everywhere
b) is fast
c) returns undef on failure instead of -1
d) is smart about variable timezones

If I had the tuits, I'd start hacking, but unfortunately I don't. I
also seem to recall long ago someone on p5p saying "I have a perfect
Time​::Local, unfortunately there isn't enough space in the margin of
my time to implement it" or something, so I'm hoping that if I didn't
imagine it, whoever that is will speak up.

That sounds like me. In fact, I did most of a rewrite (it's a very
small job), but came up against various compatibility points​:

i) returning -1 as error - unless this is changed to undef, dates
  before 1970 can't be handled.
ii) the question of what range checking should be done - the
  present partial checking clearly shouldn't be reproduced
iii) the (undocumented) "Y2k bug" handling of years​:

  if year >= 1900, use year
  if 1900 > year >= 70, use year+1900 (the correct case)
  if 70 > year, use year+2000

  Again, this makes handling of dates before 1970 impossible.

It was at the last item that my stomach rebelled, and I put the project
on one side.

I'd be happy to pick it up again, if a replacement is acceptable which

i) returns undef on error
ii) does no range checking
iii) always uses year+1900, as the spec says.

With regard to Jonathan's points​:

a) timegm() is correct everywhere
  timelocal() is correct over the range that localtime() is
  correct; in particular there's no Y2038 problem on 64 bit platforms,
  and dates before 1970 are handled correctly if localtime()
  handles negative arguments correctly.
b) should be considerably faster than the present version. In
  particular, timegm() is entirely algorithmic and needs no system
  calls.
c) returns undef on failure - this happens exactly if we are
  outside the range for which localtime() gives correct answers.
d) I'm not sure what you mean by "is smart about variable timezones";
  My code should produce correct results for any timezone which
  satisfies (i) the offset doesn't change more than once in any given
  month (ii) the offset never exceeds 13 hours.

Mike Guy

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 1999

From [Unknown Contact. See original ticket]

M . J . T . Guy <mjtg@​cus.cam.ac.uk> writes​:

d) I'm not sure what you mean by "is smart about variable timezones";
My code should produce correct results for any timezone which
satisfies (i) the offset doesn't change more than once in any given
month (ii) the offset never exceeds 13 hours.

What I think is meant about "variable timezones" is things like UK's flirting
with being GMT+1 for whole year not just in the summer. IRRC this was
the case at the 1970 epoch, so scheme which uses localtime(0) to figure
out the the base timezone fails if localtime's tables are "correct"
and return 1am.
--
Nick Ing-Simmons

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 1999

From @gsar

On Sat, 23 Oct 1999 16​:49​:10 BST, "M.J.T. Guy" wrote​:

That sounds like me. In fact, I did most of a rewrite (it's a very
small job), but came up against various compatibility points​:

i) returning -1 as error - unless this is changed to undef, dates
before 1970 can't be handled.
ii) the question of what range checking should be done - the
present partial checking clearly shouldn't be reproduced
iii) the (undocumented) "Y2k bug" handling of years​:

    if year >= 1900\,      use year
    if 1900 > year >= 70\, use year\+1900   \(the correct case\)
    if 70 > year\,         use year\+2000

 Again\, this makes handling of dates before 1970 impossible\.

It was at the last item that my stomach rebelled, and I put the project
on one side.

I'd be happy to pick it up again, if a replacement is acceptable which

i) returns undef on error
ii) does no range checking
iii) always uses year+1900, as the spec says.

#3 looks ok, but the other two will have to be based on some sort
of flag (or a differently named function). I can't put in anything
that'll obviously break existing code.

Sarathy
gsar@​ActiveState.com

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 1999

From [Unknown Contact. See original ticket]

On Sat, Oct 23, 1999 at 04​:49​:10PM +0100, M.J.T. Guy wrote​:

I'd be happy to pick it up again, if a replacement is acceptable which

i) returns undef on error
ii) does no range checking
iii) always uses year+1900, as the spec says.

That sounds great. However, breaking backward compatibility would
cause much wailing and gnashing of teeth. How about a new module name?

use NewTime​::Local;

use nTime​::Local;

use Timex​::Local; # ;-)

use XTime​::Local;

use Time​::LocalX;

use MJTG​::Time;

use Time;

Okay, so I can't come up with any good names, but maybe someone else
can. I'd prefer a new module name so that all I'd have to do to start
using it is replace the one occurance of "use Time​::Local" with
whatever the new module happens to be called.

Oh, and WRT to ii), no range checking == Good, but would you provide a
routine (or routines) that does do range checking?

d) I'm not sure what you mean by "is smart about variable timezones";
My code should produce correct results for any timezone which
satisfies (i) the offset doesn't change more than once in any given
month (ii) the offset never exceeds 13 hours.

This should be fine.

-Scott
--
Jonathan Scott Duff
duff@​cbi.tamucc.edu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant