[pve-devel] [PATCH common 1/3] fix #1963: don't do day-time related math on time stamps
Wolfgang Bumiller
w.bumiller at proxmox.com
Wed Oct 31 10:54:16 CET 2018
Since our schedules are usually written in local time, we
cannot actually perform calculations using time stamps as
for instance adding 3600 (1 hour) may yield the exact same
local time as before when translated to the current timezone
during a DST change, or might skip an hour. Thus, 2:30am + 1
hour can be all of 2:30am, 3:30am or 4:30am.
Instead, perform the translation to a "day time" array once,
then search for the scheduled time, and only at the end
translate to a time stamp again. This means adding helpers
to wrap around minutes, hours, days (of month)...
Previously, the following code looped endlessly in
compute_next_event under CEST:
my $dst_time = timelocal(0, 0, 0, 28, 9, 2018);
my $t = PVE::CalendarEvent::parse_calendar_event('mon..fri');
my $next = PVE::CalendarEvent::compute_next_event($t, $start);
Afterwards $next will be '2018-10-29 00:00 CET' as expected.
Of course, a day in which 3am appears twice with a scheduled
event for 3am will cause the event to be scheduled twice
now. Ideally we add the ability to make calendar specs use
UTC (and actually use the $utc parameter we have in
compute_next_event()). systemd.time(7) seems to allow simply
suffixing the spec with the string " UTC" for that purpose,
so we should follow this.
Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
src/PVE/CalendarEvent.pm | 174 ++++++++++++++++++++++++++++++-----------------
1 file changed, 113 insertions(+), 61 deletions(-)
diff --git a/src/PVE/CalendarEvent.pm b/src/PVE/CalendarEvent.pm
index 9fdc95a..7192712 100644
--- a/src/PVE/CalendarEvent.pm
+++ b/src/PVE/CalendarEvent.pm
@@ -161,6 +161,71 @@ sub parse_calendar_event {
return { h => $h, m => $m, dow => [ sort keys %$dow_hash ]};
}
+sub is_leap_year($) {
+ return 0 if $_[0] % 4;
+ return 1 if $_[0] % 100;
+ return 0 if $_[0] % 400;
+ return 1;
+}
+
+# mon = 0.. (Jan = 0)
+sub days_in_month($$) {
+ my ($mon, $year) = @_;
+ return 28 + is_leap_year($year) if $mon == 1;
+ return (31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31)[$mon];
+}
+
+# day = 1..
+# mon = 0.. (Jan = 0)
+sub wrap_time($) {
+ my ($time) = @_;
+ my ($sec, $min, $hour, $day, $mon, $year, $wday) = @$time;
+
+ use integer;
+ if ($sec >= 60) {
+ $min += $sec / 60;
+ $sec %= 60;
+ }
+
+ if ($min >= 60) {
+ $hour += $min / 60;
+ $min %= 60;
+ }
+
+ if ($hour >= 24) {
+ $day += $hour / 24;
+ $wday += $hour / 24;
+ $hour %= 24;
+ }
+
+ # Translate to 0..($days_in_mon-1)
+ --$day;
+ while (1) {
+ my $days_in_mon = days_in_month($mon % 12, $year);
+ last if $day < $days_in_mon;
+ # Wrap one month
+ $day -= $days_in_mon;
+ ++$mon;
+ }
+ # Translate back to 1..$days_in_mon
+ ++$day;
+
+ if ($mon >= 12) {
+ $year += $mon / 12;
+ $mon %= 12;
+ }
+
+ $wday %= 7;
+ return [$sec, $min, $hour, $day, $mon, $year, $wday];
+}
+
+# helper as we need to keep weekdays in sync
+sub time_add_days($$) {
+ my ($time, $inc) = @_;
+ my ($sec, $min, $hour, $day, $mon, $year, $wday) = @$time;
+ return wrap_time([$sec, $min, $hour, $day + $inc, $mon, $year, $wday + $inc]);
+}
+
sub compute_next_event {
my ($calspec, $last, $utc) = @_;
@@ -170,73 +235,60 @@ sub compute_next_event {
$last += 60; # at least one minute later
- while (1) {
-
- my ($min, $hour, $mday, $mon, $year, $wday);
- my $startofday;
-
- if ($utc) {
- (undef, $min, $hour, $mday, $mon, $year, $wday) = gmtime($last);
- # gmtime and timegm interpret two-digit years differently
- $year += 1900;
- $startofday = timegm(0, 0, 0, $mday, $mon, $year);
- } else {
- (undef, $min, $hour, $mday, $mon, $year, $wday) = localtime($last);
- # localtime and timelocal interpret two-digit years differently
- $year += 1900;
- $startofday = timelocal(0, 0, 0, $mday, $mon, $year);
- }
-
- $last = $startofday + $hour*3600 + $min*60;
-
- my $check_dow = sub {
- foreach my $d (@$dowspec) {
- return $last if $d == $wday;
- if ($d > $wday) {
- return $startofday + ($d-$wday)*86400;
- }
+ my $t = [$utc ? gmtime($last) : localtime($last)];
+ $t->[0] = 0; # we're not interested in seconds, actually
+ $t->[5] += 1900; # real years for clarity
+
+ outer: for (my $i = 0; $i < 1000; ++$i) {
+ my $wday = $t->[-1];
+ foreach my $d (@$dowspec) {
+ goto this_wday if $d == $wday;
+ if ($d > $wday) {
+ $t->[0] = $t->[1] = $t->[2] = 0; # sec = min = hour = 0
+ $t = time_add_days($t, $d - $wday);
+ next outer;
}
- return $startofday + (7-$wday)*86400; # start of next week
- };
-
- if ((my $next = $check_dow->()) != $last) {
- $last = $next;
- next; # repeat
}
-
- my $check_hour = sub {
- return $last if $hspec eq '*';
- foreach my $h (@$hspec) {
- return $last if $h == $hour;
- if ($h > $hour) {
- return $startofday + $h*3600;
- }
+ # Test next week:
+ $t->[0] = $t->[1] = $t->[2] = 0; # sec = min = hour = 0
+ $t = time_add_days($t, 7 - $wday);
+ next outer;
+ this_wday:
+
+ goto this_hour if $hspec eq '*';
+ my $hour = $t->[2];
+ foreach my $h (@$hspec) {
+ goto this_hour if $h == $hour;
+ if ($h > $hour) {
+ $t->[0] = $t->[1] = 0; # sec = min = 0
+ $t->[2] = $h; # hour = $h
+ next outer;
}
- return $startofday + 24*3600; # test next day
- };
-
- if ((my $next = $check_hour->()) != $last) {
- $last = $next;
- next; # repeat
}
-
- my $check_minute = sub {
- return $last if $mspec eq '*';
- foreach my $m (@$mspec) {
- return $last if $m == $min;
- if ($m > $min) {
- return $startofday +$hour*3600 + $m*60;
- }
+ # Test next day:
+ $t->[0] = $t->[1] = $t->[2] = 0; # sec = min = hour = 0
+ $t = time_add_days($t, 1);
+ next outer;
+ this_hour:
+
+ goto this_min if $mspec eq '*';
+ my $min = $t->[1];
+ foreach my $m (@$mspec) {
+ goto this_min if $m == $min;
+ if ($m > $min) {
+ $t->[0] = 0; # sec = 0
+ $t->[1] = $m; # min = $m
+ next outer;
}
- return $startofday + ($hour + 1)*3600; # test next hour
- };
-
- if ((my $next = $check_minute->()) != $last) {
- $last = $next;
- next; # repeat
- } else {
- return $last;
}
+ # Test next hour:
+ $t->[0] = $t->[1] = 0; # sec = min = hour = 0
+ $t->[2]++;
+ $t = wrap_time($t);
+ next outer;
+ this_min:
+
+ return $utc ? timegm(@$t) : timelocal(@$t);
}
die "unable to compute next calendar event\n";
--
2.11.0
More information about the pve-devel
mailing list