Zbigniew Jędrzejewski-Szmek 6384ab
From 169615c9a8cdc54d748d4dfc8279be9b3c2bec44 Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek 6384ab
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek 6384ab
Date: Sun, 21 Mar 2021 20:59:32 +0100
Zbigniew Jędrzejewski-Szmek 6384ab
Subject: [PATCH 1/5] shared/calendarspec: abort calculation after 1000
Zbigniew Jędrzejewski-Szmek 6384ab
 iterations
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
We have a bug where we seem to enter an infinite loop when running in the
Zbigniew Jędrzejewski-Szmek 6384ab
Europe/Dublin timezone. The timezone is "special" because it has negative SAVE
Zbigniew Jędrzejewski-Szmek 6384ab
values. The handling of this should obviously be fixed, but let's use a
Zbigniew Jędrzejewski-Szmek 6384ab
belt-and-suspenders approach, and gracefully fail if we fail to find an answer
Zbigniew Jędrzejewski-Szmek 6384ab
within a specific number of attempts. The code in this function is rather
Zbigniew Jędrzejewski-Szmek 6384ab
complex, and it's hard to rule out another bug in the future.
Zbigniew Jędrzejewski-Szmek 6384ab
---
Zbigniew Jędrzejewski-Szmek 6384ab
 src/shared/calendarspec.c | 14 +++++++++++++-
Zbigniew Jędrzejewski-Szmek 6384ab
 1 file changed, 13 insertions(+), 1 deletion(-)
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
index 4f68a570b52..feb43efdcda 100644
Zbigniew Jędrzejewski-Szmek 6384ab
--- a/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
+++ b/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -1210,6 +1210,10 @@ static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) {
Zbigniew Jędrzejewski-Szmek 6384ab
         return (weekdays_bits & (1 << k));
Zbigniew Jędrzejewski-Szmek 6384ab
 }
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
+/* A safety valve: if we get stuck in the calculation, return an error.
Zbigniew Jędrzejewski-Szmek 6384ab
+ * C.f. https://bugzilla.redhat.com/show_bug.cgi?id=1941335. */
Zbigniew Jędrzejewski-Szmek 6384ab
+#define MAX_CALENDAR_ITERATIONS 1000
Zbigniew Jędrzejewski-Szmek 6384ab
+
Zbigniew Jędrzejewski-Szmek 6384ab
 static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) {
Zbigniew Jędrzejewski-Szmek 6384ab
         struct tm c;
Zbigniew Jędrzejewski-Szmek 6384ab
         int tm_usec;
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -1223,7 +1227,7 @@ static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) {
Zbigniew Jędrzejewski-Szmek 6384ab
         c = *tm;
Zbigniew Jędrzejewski-Szmek 6384ab
         tm_usec = *usec;
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-        for (;;) {
Zbigniew Jędrzejewski-Szmek 6384ab
+        for (unsigned iteration = 0; iteration < MAX_CALENDAR_ITERATIONS; iteration++) {
Zbigniew Jędrzejewski-Szmek 6384ab
                 /* Normalize the current date */
Zbigniew Jędrzejewski-Szmek 6384ab
                 (void) mktime_or_timegm(&c, spec->utc);
Zbigniew Jędrzejewski-Szmek 6384ab
                 c.tm_isdst = spec->dst;
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -1320,6 +1324,14 @@ static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) {
Zbigniew Jędrzejewski-Szmek 6384ab
                 *usec = tm_usec;
Zbigniew Jędrzejewski-Szmek 6384ab
                 return 0;
Zbigniew Jędrzejewski-Szmek 6384ab
         }
Zbigniew Jędrzejewski-Szmek 6384ab
+
Zbigniew Jędrzejewski-Szmek 6384ab
+        /* It seems we entered an infinite loop. Let's gracefully return an error instead of hanging or
Zbigniew Jędrzejewski-Szmek 6384ab
+         * aborting. This code is also exercised when timers.target is brought up during early boot, so
Zbigniew Jędrzejewski-Szmek 6384ab
+         * aborting here is problematic and hard to diagnose for users. */
Zbigniew Jędrzejewski-Szmek 6384ab
+        _cleanup_free_ char *s = NULL;
Zbigniew Jędrzejewski-Szmek 6384ab
+        (void) calendar_spec_to_string(spec, &s);
Zbigniew Jędrzejewski-Szmek 6384ab
+        return log_warning_errno(SYNTHETIC_ERRNO(EDEADLK),
Zbigniew Jędrzejewski-Szmek 6384ab
+                                 "Infinite loop in calendar calculation: %s", strna(s));
Zbigniew Jędrzejewski-Szmek 6384ab
 }
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
 static int calendar_spec_next_usec_impl(const CalendarSpec *spec, usec_t usec, usec_t *ret_next) {
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
From 462f15d92d35f812d7d77edd486ca63236cffe83 Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek 6384ab
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek 6384ab
Date: Mon, 22 Mar 2021 09:20:47 +0100
Zbigniew Jędrzejewski-Szmek 6384ab
Subject: [PATCH 2/5] shared/calendarspec: constify parameter and simplify
Zbigniew Jędrzejewski-Szmek 6384ab
 assignments to variable
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
The scope of start & stop is narrowed down, and they are assigned only once.
Zbigniew Jędrzejewski-Szmek 6384ab
No functional change, but I think the code is easier to read this way.
Zbigniew Jędrzejewski-Szmek 6384ab
Also add a comment to make the code easier to read.
Zbigniew Jędrzejewski-Szmek 6384ab
---
Zbigniew Jędrzejewski-Szmek 6384ab
 src/shared/calendarspec.c | 33 ++++++++++++++++++++++-----------
Zbigniew Jędrzejewski-Szmek 6384ab
 1 file changed, 22 insertions(+), 11 deletions(-)
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
index feb43efdcda..5c666412946 100644
Zbigniew Jędrzejewski-Szmek 6384ab
--- a/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
+++ b/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -1101,7 +1101,7 @@ int calendar_spec_from_string(const char *p, CalendarSpec **spec) {
Zbigniew Jędrzejewski-Szmek 6384ab
         return 0;
Zbigniew Jędrzejewski-Szmek 6384ab
 }
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-static int find_end_of_month(struct tm *tm, bool utc, int day) {
Zbigniew Jędrzejewski-Szmek 6384ab
+static int find_end_of_month(const struct tm *tm, bool utc, int day) {
Zbigniew Jędrzejewski-Szmek 6384ab
         struct tm t = *tm;
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         t.tm_mon++;
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -1114,28 +1114,39 @@ static int find_end_of_month(struct tm *tm, bool utc, int day) {
Zbigniew Jędrzejewski-Szmek 6384ab
         return t.tm_mday;
Zbigniew Jędrzejewski-Szmek 6384ab
 }
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-static int find_matching_component(const CalendarSpec *spec, const CalendarComponent *c,
Zbigniew Jędrzejewski-Szmek 6384ab
-                                   struct tm *tm, int *val) {
Zbigniew Jędrzejewski-Szmek 6384ab
-        const CalendarComponent *p = c;
Zbigniew Jędrzejewski-Szmek 6384ab
-        int start, stop, d = -1;
Zbigniew Jędrzejewski-Szmek 6384ab
+static int find_matching_component(
Zbigniew Jędrzejewski-Szmek 6384ab
+                const CalendarSpec *spec,
Zbigniew Jędrzejewski-Szmek 6384ab
+                const CalendarComponent *c,
Zbigniew Jędrzejewski-Szmek 6384ab
+                const struct tm *tm,           /* tm is only used for end-of-month calculations */
Zbigniew Jędrzejewski-Szmek 6384ab
+                int *val) {
Zbigniew Jędrzejewski-Szmek 6384ab
+
Zbigniew Jędrzejewski-Szmek 6384ab
+        int d = -1, r;
Zbigniew Jędrzejewski-Szmek 6384ab
         bool d_set = false;
Zbigniew Jędrzejewski-Szmek 6384ab
-        int r;
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert(val);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
+        /* Finds the *earliest* matching time specified by one of the CalendarCompoment items in chain c.
Zbigniew Jędrzejewski-Szmek 6384ab
+         * If no matches can be found, returns -ENOENT.
Zbigniew Jędrzejewski-Szmek 6384ab
+         * Otherwise, updates *val to the matching time. 1 is returned if *val was changed, 0 otherwise.
Zbigniew Jędrzejewski-Szmek 6384ab
+         */
Zbigniew Jędrzejewski-Szmek 6384ab
+
Zbigniew Jędrzejewski-Szmek 6384ab
         if (!c)
Zbigniew Jędrzejewski-Szmek 6384ab
                 return 0;
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
+        bool end_of_month = spec->end_of_month && c == spec->day;
Zbigniew Jędrzejewski-Szmek 6384ab
+
Zbigniew Jędrzejewski-Szmek 6384ab
         while (c) {
Zbigniew Jędrzejewski-Szmek 6384ab
-                start = c->start;
Zbigniew Jędrzejewski-Szmek 6384ab
-                stop = c->stop;
Zbigniew Jędrzejewski-Szmek 6384ab
+                int start, stop;
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-                if (spec->end_of_month && p == spec->day) {
Zbigniew Jędrzejewski-Szmek 6384ab
-                        start = find_end_of_month(tm, spec->utc, start);
Zbigniew Jędrzejewski-Szmek 6384ab
-                        stop = find_end_of_month(tm, spec->utc, stop);
Zbigniew Jędrzejewski-Szmek 6384ab
+                if (end_of_month) {
Zbigniew Jędrzejewski-Szmek 6384ab
+                        start = find_end_of_month(tm, spec->utc, c->start);
Zbigniew Jędrzejewski-Szmek 6384ab
+                        stop = find_end_of_month(tm, spec->utc, c->stop);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
                         if (stop > 0)
Zbigniew Jędrzejewski-Szmek 6384ab
                                 SWAP_TWO(start, stop);
Zbigniew Jędrzejewski-Szmek 6384ab
+                } else {
Zbigniew Jędrzejewski-Szmek 6384ab
+                        start = c->start;
Zbigniew Jędrzejewski-Szmek 6384ab
+                        stop = c->stop;
Zbigniew Jędrzejewski-Szmek 6384ab
                 }
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
                 if (start >= *val) {
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
From f035bb1b7a5900439640f267db881c60d042e450 Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek 6384ab
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek 6384ab
Date: Mon, 22 Mar 2021 11:10:22 +0100
Zbigniew Jędrzejewski-Szmek 6384ab
Subject: [PATCH 3/5] test-calendarspec: print offending line in output
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
The output is rather long at this makes it easier to jump to the right place.
Zbigniew Jędrzejewski-Szmek 6384ab
Also use normal output routines and set_unset_env() to make things more
Zbigniew Jędrzejewski-Szmek 6384ab
compact.
Zbigniew Jędrzejewski-Szmek 6384ab
---
Zbigniew Jędrzejewski-Szmek 6384ab
 src/test/test-calendarspec.c | 48 +++++++++++++++++-------------------
Zbigniew Jędrzejewski-Szmek 6384ab
 1 file changed, 22 insertions(+), 26 deletions(-)
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
diff --git a/src/test/test-calendarspec.c b/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
index 01ec7f87704..152ce879f8a 100644
Zbigniew Jędrzejewski-Szmek 6384ab
--- a/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
+++ b/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -2,11 +2,11 @@
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
 #include "alloc-util.h"
Zbigniew Jędrzejewski-Szmek 6384ab
 #include "calendarspec.h"
Zbigniew Jędrzejewski-Szmek 6384ab
+#include "env-util.h"
Zbigniew Jędrzejewski-Szmek 6384ab
 #include "errno-util.h"
Zbigniew Jędrzejewski-Szmek 6384ab
 #include "string-util.h"
Zbigniew Jędrzejewski-Szmek 6384ab
-#include "util.h"
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-static void test_one(const char *input, const char *output) {
Zbigniew Jędrzejewski-Szmek 6384ab
+static void _test_one(int line, const char *input, const char *output) {
Zbigniew Jędrzejewski-Szmek 6384ab
         CalendarSpec *c;
Zbigniew Jędrzejewski-Szmek 6384ab
         _cleanup_free_ char *p = NULL, *q = NULL;
Zbigniew Jędrzejewski-Szmek 6384ab
         usec_t u;
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -16,13 +16,13 @@ static void test_one(const char *input, const char *output) {
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(calendar_spec_from_string(input, &c) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(calendar_spec_to_string(c, &p) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("\"%s\" → \"%s\"\n", input, p);
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("line %d: \"%s\" → \"%s\"", line, input, p);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(streq(p, output));
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         u = now(CLOCK_REALTIME);
Zbigniew Jędrzejewski-Szmek 6384ab
         r = calendar_spec_next_usec(c, u, &u);
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("Next: %s\n", r < 0 ? strerror_safe(r) : format_timestamp(buf, sizeof(buf), u));
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("Next: %s", r < 0 ? strerror_safe(r) : format_timestamp(buf, sizeof buf, u));
Zbigniew Jędrzejewski-Szmek 6384ab
         calendar_spec_free(c);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(calendar_spec_from_string(p, &c) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -31,8 +31,9 @@ static void test_one(const char *input, const char *output) {
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(streq(q, p));
Zbigniew Jędrzejewski-Szmek 6384ab
 }
Zbigniew Jędrzejewski-Szmek 6384ab
+#define test_one(input, output) _test_one(__LINE__, input, output)
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-static void test_next(const char *input, const char *new_tz, usec_t after, usec_t expect) {
Zbigniew Jędrzejewski-Szmek 6384ab
+static void _test_next(int line, const char *input, const char *new_tz, usec_t after, usec_t expect) {
Zbigniew Jędrzejewski-Szmek 6384ab
         CalendarSpec *c;
Zbigniew Jędrzejewski-Szmek 6384ab
         usec_t u;
Zbigniew Jędrzejewski-Szmek 6384ab
         char *old_tz;
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -43,22 +44,19 @@ static void test_next(const char *input, const char *new_tz, usec_t after, usec_
Zbigniew Jędrzejewski-Szmek 6384ab
         if (old_tz)
Zbigniew Jędrzejewski-Szmek 6384ab
                 old_tz = strdupa(old_tz);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-        if (new_tz) {
Zbigniew Jędrzejewski-Szmek 6384ab
-                char *colon_tz;
Zbigniew Jędrzejewski-Szmek 6384ab
+        if (new_tz)
Zbigniew Jędrzejewski-Szmek 6384ab
+                new_tz = strjoina(":", new_tz);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-                colon_tz = strjoina(":", new_tz);
Zbigniew Jędrzejewski-Szmek 6384ab
-                assert_se(setenv("TZ", colon_tz, 1) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
-        } else
Zbigniew Jędrzejewski-Szmek 6384ab
-                assert_se(unsetenv("TZ") >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
+        assert_se(set_unset_env("TZ", new_tz, true) == 0);
Zbigniew Jędrzejewski-Szmek 6384ab
         tzset();
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(calendar_spec_from_string(input, &c) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("\"%s\"\n", input);
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("line %d: \"%s\" new_tz=%s", line, input, strnull(new_tz));
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         u = after;
Zbigniew Jędrzejewski-Szmek 6384ab
         r = calendar_spec_next_usec(c, after, &u);
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("At: %s\n", r < 0 ? strerror_safe(r) : format_timestamp_style(buf, sizeof buf, u, TIMESTAMP_US));
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("At: %s", r < 0 ? strerror_safe(r) : format_timestamp_style(buf, sizeof buf, u, TIMESTAMP_US));
Zbigniew Jędrzejewski-Szmek 6384ab
         if (expect != USEC_INFINITY)
Zbigniew Jędrzejewski-Szmek 6384ab
                 assert_se(r >= 0 && u == expect);
Zbigniew Jędrzejewski-Szmek 6384ab
         else
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -66,12 +64,10 @@ static void test_next(const char *input, const char *new_tz, usec_t after, usec_
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         calendar_spec_free(c);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-        if (old_tz)
Zbigniew Jędrzejewski-Szmek 6384ab
-                assert_se(setenv("TZ", old_tz, 1) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
-        else
Zbigniew Jędrzejewski-Szmek 6384ab
-                assert_se(unsetenv("TZ") >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
+        assert_se(set_unset_env("TZ", old_tz, true) == 0);
Zbigniew Jędrzejewski-Szmek 6384ab
         tzset();
Zbigniew Jędrzejewski-Szmek 6384ab
 }
Zbigniew Jędrzejewski-Szmek 6384ab
+#define test_next(input, new_tz, after, expect) _test_next(__LINE__, input,new_tz,after,expect)
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
 static void test_timestamp(void) {
Zbigniew Jędrzejewski-Szmek 6384ab
         char buf[FORMAT_TIMESTAMP_MAX];
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -83,12 +79,12 @@ static void test_timestamp(void) {
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         x = now(CLOCK_REALTIME);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-        assert_se(format_timestamp_style(buf, sizeof(buf), x, TIMESTAMP_US));
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("%s\n", buf);
Zbigniew Jędrzejewski-Szmek 6384ab
+        assert_se(format_timestamp_style(buf, sizeof buf, x, TIMESTAMP_US));
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("%s", buf);
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(calendar_spec_from_string(buf, &c) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(calendar_spec_to_string(c, &t) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
         calendar_spec_free(c);
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("%s\n", t);
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("%s", t);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(parse_timestamp(t, &y) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(y == x);
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -104,11 +100,11 @@ static void test_hourly_bug_4031(void) {
Zbigniew Jędrzejewski-Szmek 6384ab
         n = now(CLOCK_REALTIME);
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se((r = calendar_spec_next_usec(c, n, &u)) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("Now: %s (%"PRIu64")\n", format_timestamp_style(buf, sizeof buf, n, TIMESTAMP_US), n);
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("Next hourly: %s (%"PRIu64")\n", r < 0 ? strerror_safe(r) : format_timestamp_style(buf, sizeof buf, u, TIMESTAMP_US), u);
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("Now: %s (%"PRIu64")", format_timestamp_style(buf, sizeof buf, n, TIMESTAMP_US), n);
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("Next hourly: %s (%"PRIu64")", r < 0 ? strerror_safe(r) : format_timestamp_style(buf, sizeof buf, u, TIMESTAMP_US), u);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se((r = calendar_spec_next_usec(c, u, &w)) >= 0);
Zbigniew Jędrzejewski-Szmek 6384ab
-        printf("Next hourly: %s (%"PRIu64")\n", r < 0 ? strerror_safe(r) : format_timestamp_style(zaf, sizeof zaf, w, TIMESTAMP_US), w);
Zbigniew Jędrzejewski-Szmek 6384ab
+        log_info("Next hourly: %s (%"PRIu64")", r < 0 ? strerror_safe(r) : format_timestamp_style(zaf, sizeof zaf, w, TIMESTAMP_US), w);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(n < u);
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(u <= n + USEC_PER_HOUR);
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -209,13 +205,13 @@ int main(int argc, char* argv[]) {
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-08-06 9..17/2:00 UTC", "", 1502029800000000, 1502031600000000);
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2016-12-* 3..21/6:00 UTC", "", 1482613200000001, 1482634800000000);
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-09-24 03:30:00 Pacific/Auckland", "", 12345, 1506177000000000);
Zbigniew Jędrzejewski-Szmek 6384ab
-        // Due to daylight saving time - 2017-09-24 02:30:00 does not exist
Zbigniew Jędrzejewski-Szmek 6384ab
+        /* Due to daylight saving time - 2017-09-24 02:30:00 does not exist */
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-09-24 02:30:00 Pacific/Auckland", "", 12345, -1);
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-04-02 02:30:00 Pacific/Auckland", "", 12345, 1491053400000000);
Zbigniew Jędrzejewski-Szmek 6384ab
-        // Confirm that even though it's a time change here (backward) 02:30 happens only once
Zbigniew Jędrzejewski-Szmek 6384ab
+        /* Confirm that even though it's a time change here (backward) 02:30 happens only once */
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-04-02 02:30:00 Pacific/Auckland", "", 1491053400000000, -1);
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-04-02 03:30:00 Pacific/Auckland", "", 12345, 1491060600000000);
Zbigniew Jędrzejewski-Szmek 6384ab
-        // Confirm that timezones in the Spec work regardless of current timezone
Zbigniew Jędrzejewski-Szmek 6384ab
+        /* Confirm that timezones in the Spec work regardless of current timezone */
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-09-09 20:42:00 Pacific/Auckland", "", 12345, 1504946520000000);
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-09-09 20:42:00 Pacific/Auckland", "EET", 12345, 1504946520000000);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
From 47b0b65766229a18921a3ce831ef708ef408a34c Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek 6384ab
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek 6384ab
Date: Mon, 22 Mar 2021 11:29:35 +0100
Zbigniew Jędrzejewski-Szmek 6384ab
Subject: [PATCH 4/5] test-calendarspec: do not convert timezone "" to ":"
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
I *think* it doesn't actually make any difference, because ":" will be ignored.
Zbigniew Jędrzejewski-Szmek 6384ab
437f48a471f51ac9dd2697ee3b848a71b4f101df added prefixing with ":", but didn't
Zbigniew Jędrzejewski-Szmek 6384ab
take into account the fact that we also use "" with a different meaning than
Zbigniew Jędrzejewski-Szmek 6384ab
NULL here. But let's restore the original behaviour of specifying the empty
Zbigniew Jędrzejewski-Szmek 6384ab
string.
Zbigniew Jędrzejewski-Szmek 6384ab
---
Zbigniew Jędrzejewski-Szmek 6384ab
 src/test/test-calendarspec.c | 2 +-
Zbigniew Jędrzejewski-Szmek 6384ab
 1 file changed, 1 insertion(+), 1 deletion(-)
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
diff --git a/src/test/test-calendarspec.c b/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
index 152ce879f8a..c62e6860cf9 100644
Zbigniew Jędrzejewski-Szmek 6384ab
--- a/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
+++ b/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -44,7 +44,7 @@ static void _test_next(int line, const char *input, const char *new_tz, usec_t a
Zbigniew Jędrzejewski-Szmek 6384ab
         if (old_tz)
Zbigniew Jędrzejewski-Szmek 6384ab
                 old_tz = strdupa(old_tz);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
-        if (new_tz)
Zbigniew Jędrzejewski-Szmek 6384ab
+        if (!isempty(new_tz))
Zbigniew Jędrzejewski-Szmek 6384ab
                 new_tz = strjoina(":", new_tz);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(set_unset_env("TZ", new_tz, true) == 0);
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
From 129cb6e249bef30dc33e08f98f0b27a6de976f6f Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek 6384ab
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek 6384ab
Date: Mon, 22 Mar 2021 12:51:47 +0100
Zbigniew Jędrzejewski-Szmek 6384ab
Subject: [PATCH 5/5] shared/calendarspec: when mktime() moves us backwards,
Zbigniew Jędrzejewski-Szmek 6384ab
 jump forward
Zbigniew Jędrzejewski-Szmek 6384ab
MIME-Version: 1.0
Zbigniew Jędrzejewski-Szmek 6384ab
Content-Type: text/plain; charset=UTF-8
Zbigniew Jędrzejewski-Szmek 6384ab
Content-Transfer-Encoding: 8bit
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
When trying to calculate the next firing of 'Sun *-*-* 01:00:00', we'd fall
Zbigniew Jędrzejewski-Szmek 6384ab
into an infinite loop, because mktime() moves us "backwards":
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
Before this patch:
Zbigniew Jędrzejewski-Szmek 6384ab
tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
Zbigniew Jędrzejewski-Szmek 6384ab
tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
Zbigniew Jędrzejewski-Szmek 6384ab
tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
Zbigniew Jędrzejewski-Szmek 6384ab
...
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
We rely on mktime() normalizing the time. The man page does not say that it'll
Zbigniew Jędrzejewski-Szmek 6384ab
move the time forward, but our algorithm relies on this. So let's catch this
Zbigniew Jędrzejewski-Szmek 6384ab
case explicitly.
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
With this patch:
Zbigniew Jędrzejewski-Szmek 6384ab
$ TZ=Europe/Dublin faketime 2021-03-21 build/systemd-analyze calendar --iterations=5 'Sun *-*-* 01:00:00'
Zbigniew Jędrzejewski-Szmek 6384ab
Normalized form: Sun *-*-* 01:00:00
Zbigniew Jędrzejewski-Szmek 6384ab
    Next elapse: Sun 2021-03-21 01:00:00 GMT
Zbigniew Jędrzejewski-Szmek 6384ab
       (in UTC): Sun 2021-03-21 01:00:00 UTC
Zbigniew Jędrzejewski-Szmek 6384ab
       From now: 59min left
Zbigniew Jędrzejewski-Szmek 6384ab
       Iter. #2: Sun 2021-04-04 01:00:00 IST
Zbigniew Jędrzejewski-Szmek 6384ab
       (in UTC): Sun 2021-04-04 00:00:00 UTC
Zbigniew Jędrzejewski-Szmek 6384ab
       From now: 1 weeks 6 days left           <---- note the 2 week jump here
Zbigniew Jędrzejewski-Szmek 6384ab
       Iter. #3: Sun 2021-04-11 01:00:00 IST
Zbigniew Jędrzejewski-Szmek 6384ab
       (in UTC): Sun 2021-04-11 00:00:00 UTC
Zbigniew Jędrzejewski-Szmek 6384ab
       From now: 2 weeks 6 days left
Zbigniew Jędrzejewski-Szmek 6384ab
       Iter. #4: Sun 2021-04-18 01:00:00 IST
Zbigniew Jędrzejewski-Szmek 6384ab
       (in UTC): Sun 2021-04-18 00:00:00 UTC
Zbigniew Jędrzejewski-Szmek 6384ab
       From now: 3 weeks 6 days left
Zbigniew Jędrzejewski-Szmek 6384ab
       Iter. #5: Sun 2021-04-25 01:00:00 IST
Zbigniew Jędrzejewski-Szmek 6384ab
       (in UTC): Sun 2021-04-25 00:00:00 UTC
Zbigniew Jędrzejewski-Szmek 6384ab
       From now: 1 months 4 days left
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1941335.
Zbigniew Jędrzejewski-Szmek 6384ab
---
Zbigniew Jędrzejewski-Szmek 6384ab
 src/shared/calendarspec.c    | 19 +++++++++++--------
Zbigniew Jędrzejewski-Szmek 6384ab
 src/test/test-calendarspec.c |  3 +++
Zbigniew Jędrzejewski-Szmek 6384ab
 test/test-functions          |  1 +
Zbigniew Jędrzejewski-Szmek 6384ab
 3 files changed, 15 insertions(+), 8 deletions(-)
Zbigniew Jędrzejewski-Szmek 6384ab
Zbigniew Jędrzejewski-Szmek 6384ab
diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
index 5c666412946..bf24d8d5bbb 100644
Zbigniew Jędrzejewski-Szmek 6384ab
--- a/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
+++ b/src/shared/calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -1195,15 +1195,18 @@ static int tm_within_bounds(struct tm *tm, bool utc) {
Zbigniew Jędrzejewski-Szmek 6384ab
                 return negative_errno();
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         /* Did any normalization take place? If so, it was out of bounds before */
Zbigniew Jędrzejewski-Szmek 6384ab
-        bool good = t.tm_year == tm->tm_year &&
Zbigniew Jędrzejewski-Szmek 6384ab
-                    t.tm_mon  == tm->tm_mon  &&
Zbigniew Jędrzejewski-Szmek 6384ab
-                    t.tm_mday == tm->tm_mday &&
Zbigniew Jędrzejewski-Szmek 6384ab
-                    t.tm_hour == tm->tm_hour &&
Zbigniew Jędrzejewski-Szmek 6384ab
-                    t.tm_min  == tm->tm_min  &&
Zbigniew Jędrzejewski-Szmek 6384ab
-                    t.tm_sec  == tm->tm_sec;
Zbigniew Jędrzejewski-Szmek 6384ab
-        if (!good)
Zbigniew Jędrzejewski-Szmek 6384ab
+        int cmp = CMP(t.tm_year, tm->tm_year) ?:
Zbigniew Jędrzejewski-Szmek 6384ab
+                  CMP(t.tm_mon, tm->tm_mon) ?:
Zbigniew Jędrzejewski-Szmek 6384ab
+                  CMP(t.tm_mday, tm->tm_mday) ?:
Zbigniew Jędrzejewski-Szmek 6384ab
+                  CMP(t.tm_hour, tm->tm_hour) ?:
Zbigniew Jędrzejewski-Szmek 6384ab
+                  CMP(t.tm_min, tm->tm_min) ?:
Zbigniew Jędrzejewski-Szmek 6384ab
+                  CMP(t.tm_sec, tm->tm_sec);
Zbigniew Jędrzejewski-Szmek 6384ab
+
Zbigniew Jędrzejewski-Szmek 6384ab
+        if (cmp < 0)
Zbigniew Jędrzejewski-Szmek 6384ab
+                return -EDEADLK; /* Refuse to go backward */
Zbigniew Jędrzejewski-Szmek 6384ab
+        if (cmp > 0)
Zbigniew Jędrzejewski-Szmek 6384ab
                 *tm = t;
Zbigniew Jędrzejewski-Szmek 6384ab
-        return good;
Zbigniew Jędrzejewski-Szmek 6384ab
+        return cmp == 0;
Zbigniew Jędrzejewski-Szmek 6384ab
 }
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
 static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) {
Zbigniew Jędrzejewski-Szmek 6384ab
diff --git a/src/test/test-calendarspec.c b/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
index c62e6860cf9..4f1d0f64d57 100644
Zbigniew Jędrzejewski-Szmek 6384ab
--- a/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
+++ b/src/test/test-calendarspec.c
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -214,6 +214,9 @@ int main(int argc, char* argv[]) {
Zbigniew Jędrzejewski-Szmek 6384ab
         /* Confirm that timezones in the Spec work regardless of current timezone */
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-09-09 20:42:00 Pacific/Auckland", "", 12345, 1504946520000000);
Zbigniew Jędrzejewski-Szmek 6384ab
         test_next("2017-09-09 20:42:00 Pacific/Auckland", "EET", 12345, 1504946520000000);
Zbigniew Jędrzejewski-Szmek 6384ab
+        /* Check that we don't start looping if mktime() moves us backwards */
Zbigniew Jędrzejewski-Szmek 6384ab
+        test_next("Sun *-*-* 01:00:00 Europe/Dublin", "", 1616412478000000, 1617494400000000);
Zbigniew Jędrzejewski-Szmek 6384ab
+        test_next("Sun *-*-* 01:00:00 Europe/Dublin", "IST", 1616412478000000, 1617494400000000);
Zbigniew Jędrzejewski-Szmek 6384ab
 
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(calendar_spec_from_string("test", &c) < 0);
Zbigniew Jędrzejewski-Szmek 6384ab
         assert_se(calendar_spec_from_string(" utc", &c) < 0);
Zbigniew Jędrzejewski-Szmek 6384ab
diff --git a/test/test-functions b/test/test-functions
Zbigniew Jędrzejewski-Szmek 6384ab
index d7f7967e2ff..6b94058fd36 100644
Zbigniew Jędrzejewski-Szmek 6384ab
--- a/test/test-functions
Zbigniew Jędrzejewski-Szmek 6384ab
+++ b/test/test-functions
Zbigniew Jędrzejewski-Szmek 6384ab
@@ -1340,6 +1340,7 @@ install_zoneinfo() {
Zbigniew Jędrzejewski-Szmek 6384ab
     inst_any /usr/share/zoneinfo/Asia/Vladivostok
Zbigniew Jędrzejewski-Szmek 6384ab
     inst_any /usr/share/zoneinfo/Australia/Sydney
Zbigniew Jędrzejewski-Szmek 6384ab
     inst_any /usr/share/zoneinfo/Europe/Berlin
Zbigniew Jędrzejewski-Szmek 6384ab
+    inst_any /usr/share/zoneinfo/Europe/Dublin
Zbigniew Jędrzejewski-Szmek 6384ab
     inst_any /usr/share/zoneinfo/Europe/Kiev
Zbigniew Jędrzejewski-Szmek 6384ab
     inst_any /usr/share/zoneinfo/Pacific/Auckland
Zbigniew Jędrzejewski-Szmek 6384ab
     inst_any /usr/share/zoneinfo/Pacific/Honolulu