time-util: fix issues in parse_timestamp() and optimize performance

Previously, an input string ends with short timezone spec e.g. WET,
was parsed by setting $TZ environment variable to the timezone.
But the timezone might be different from the original local timezone,
thus the result might not follow the timezone change in the original
local timezone.

This makes the check of the short timezone spec with tzname[] earlier,
then it is not necessary to load another timezone file for e.g. WET,
and provides expected time.

This also make it use SAVE_TIMEZONE macro and drop use of forking
process. This makes greatly improve performance when parsing string
that contains timezone different from the current local timezone.

Unfortunately, there is still one corner case that our test fails.
When tzdata is built with rearguard enabled, then at least
Africa/Windhoek timezone does not provide correct time, but time shifted
1 hour from the original.

diff --git a/src/basic/time-util.c b/src/basic/time-util.c
index fa24d79d53ea167714fe0332bf98e3a703e99926..fb54ba276c1d4177283cf2ab42742915f86db14d 100644 (file)
--- a/src/basic/time-util.c
+++ b/src/basic/time-util.c
@@ -966,44 +966,13 @@ finish:
         return 0;
 }
 
-static int parse_timestamp_maybe_with_tz(const char *t, size_t tz_offset, bool valid_tz, usec_t *ret) {
-        assert(t);
-
-        tzset();
-
-        for (int j = 0; j <= 1; j++) {
-                if (isempty(tzname[j]))
-                        continue;
-
-                if (!streq(t + tz_offset, tzname[j]))
-                        continue;
-
-                /* The specified timezone matches tzname[] of the local timezone. */
-                return parse_timestamp_impl(t, tz_offset - 1, /* utc = */ false, /* isdst = */ j, /* gmtoff = */ 0, ret);
-        }
-
-        /* If we know that the last word is a valid timezone (e.g. Asia/Tokyo), then simply drop the timezone
-         * and parse the remaining string as a local time. If we know that the last word is not a timezone,
-         * then assume that it is a part of the time and try to parse the whole string as a local time. */
-        return parse_timestamp_impl(t, valid_tz ? tz_offset - 1 : SIZE_MAX,
-                                    /* utc = */ false, /* isdst = */ -1, /* gmtoff = */ 0, ret);
-}
-
-typedef struct ParseTimestampResult {
-        usec_t usec;
-        int return_value;
-} ParseTimestampResult;
-
 int parse_timestamp(const char *t, usec_t *ret) {
-        ParseTimestampResult *shared, tmp;
-        const char *k, *tz, *current_tz;
-        size_t max_len, t_len;
-        struct tm tm;
+        long gmtoff;
         int r;
 
         assert(t);
 
-        t_len = strlen(t);
+        size_t t_len = strlen(t);
         if (t_len > 2 && t[t_len - 1] == 'Z') {
                 /* Try to parse as RFC3339-style welded UTC: "1985-04-12T23:20:50.52Z" */
                 r = parse_timestamp_impl(t, t_len - 1, /* utc = */ true, /* isdst = */ -1, /* gmtoff = */ 0, ret);
@@ -1011,17 +980,15 @@ int parse_timestamp(const char *t, usec_t *ret) {
                         return r;
         }
 
-        if (t_len > 7 && IN_SET(t[t_len - 6], '+', '-') && t[t_len - 7] != ' ') {  /* RFC3339-style welded offset: "1990-12-31T15:59:60-08:00" */
-                k = strptime(&t[t_len - 6], "%z", &tm);
-                if (k && *k == '\0')
-                        return parse_timestamp_impl(t, t_len - 6, /* utc = */ true, /* isdst = */ -1, /* gmtoff = */ tm.tm_gmtoff, ret);
-        }
+        /* RFC3339-style welded offset: "1990-12-31T15:59:60-08:00" */
+        if (t_len > 7 && IN_SET(t[t_len - 6], '+', '-') && t[t_len - 7] != ' ' && parse_gmtoff(&t[t_len - 6], &gmtoff) >= 0)
+                return parse_timestamp_impl(t, t_len - 6, /* utc = */ true, /* isdst = */ -1, gmtoff, ret);
 
-        tz = strrchr(t, ' ');
+        const char *tz = strrchr(t, ' ');
         if (!tz)
                 return parse_timestamp_impl(t, /* max_len = */ SIZE_MAX, /* utc = */ false, /* isdst = */ -1, /* gmtoff = */ 0, ret);
 
-        max_len = tz - t;
+        size_t max_len = tz - t;
         tz++;
 
         /* Shortcut, parse the string as UTC. */
@@ -1031,65 +998,39 @@ int parse_timestamp(const char *t, usec_t *ret) {
         /* If the timezone is compatible with RFC-822/ISO 8601 (e.g. +06, or -03:00) then parse the string as
          * UTC and shift the result. Note, this must be earlier than the timezone check with tzname[], as
          * tzname[] may be in the same format. */
-        k = strptime(tz, "%z", &tm);
-        if (k && *k == '\0')
-                return parse_timestamp_impl(t, max_len, /* utc = */ true, /* isdst = */ -1, /* gmtoff = */ tm.tm_gmtoff, ret);
-
-        /* If the last word is not a timezone file (e.g. Asia/Tokyo), then let's check if it matches
-         * tzname[] of the local timezone, e.g. JST or CEST. */
-        if (!timezone_is_valid(tz, LOG_DEBUG))
-                return parse_timestamp_maybe_with_tz(t, tz - t, /* valid_tz = */ false, ret);
-
-        /* Shortcut. If the current $TZ is equivalent to the specified timezone, it is not necessary to fork
-         * the process. */
-        current_tz = getenv("TZ");
-        if (current_tz && *current_tz == ':' && streq(current_tz + 1, tz))
-                return parse_timestamp_maybe_with_tz(t, tz - t, /* valid_tz = */ true, ret);
-
-        /* Otherwise, to avoid polluting the current environment variables, let's fork the process and set
-         * the specified timezone in the child process. */
-
-        shared = mmap(NULL, sizeof *shared, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
-        if (shared == MAP_FAILED)
-                return negative_errno();
-
-        /* The input string may be in argv. Let's copy it. */
-        _cleanup_free_ char *t_copy = strdup(t);
-        if (!t_copy)
-                return -ENOMEM;
-
-        t = t_copy;
-        assert_se(tz = endswith(t_copy, tz));
-
-        r = safe_fork("(sd-timestamp)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGKILL|FORK_WAIT, NULL);
-        if (r < 0) {
-                (void) munmap(shared, sizeof *shared);
-                return r;
-        }
-        if (r == 0) {
-                const char *colon_tz;
-
-                /* tzset(3) says $TZ should be prefixed with ":" if we reference timezone files */
-                colon_tz = strjoina(":", tz);
+        if (parse_gmtoff(tz, &gmtoff) >= 0)
+                return parse_timestamp_impl(t, max_len, /* utc = */ true, /* isdst = */ -1, gmtoff, ret);
 
-                if (setenv("TZ", colon_tz, 1) != 0) {
-                        shared->return_value = negative_errno();
-                        _exit(EXIT_FAILURE);
-                }
+        /* Check if the last word matches tzname[] of the local timezone. Note, this must be done earlier
+         * than the check by timezone_is_valid() below, as some short timezone specifications have their own
+         * timezone files (e.g. WET has its own timezone file, but JST does not), but using such files does
+         * not follow the timezone change in the current area. */
+        tzset();
+        for (int j = 0; j <= 1; j++) {
+                if (isempty(tzname[j]))
+                        continue;
 
-                shared->return_value = parse_timestamp_maybe_with_tz(t, tz - t, /* valid_tz = */ true, &shared->usec);
+                if (!streq(tz, tzname[j]))
+                        continue;
 
-                _exit(EXIT_SUCCESS);
+                /* The specified timezone matches tzname[] of the local timezone. */
+                return parse_timestamp_impl(t, max_len, /* utc = */ false, /* isdst = */ j, /* gmtoff = */ 0, ret);
         }
 
-        tmp = *shared;
-        if (munmap(shared, sizeof *shared) != 0)
-                return negative_errno();
+        /* If the last word is a valid timezone file (e.g. Asia/Tokyo), then save the current timezone, apply
+         * the specified timezone, and parse the remaining string as a local time. */
+        if (timezone_is_valid(tz, LOG_DEBUG)) {
+                SAVE_TIMEZONE;
+
+                if (setenv("TZ", strjoina(":", tz), /* overwrite = */ true) < 0)
+                        return negative_errno();
 
-        if (tmp.return_value == 0 && ret)
-                *ret = tmp.usec;
+                return parse_timestamp_impl(t, max_len, /* utc = */ false, /* isdst = */ -1, /* gmtoff = */ 0, ret);
+        }
 
-        return tmp.return_value;
+        /* Otherwise, assume that the last word is a part of the time and try to parse the whole string as a
+         * local time. */
+        return parse_timestamp_impl(t, SIZE_MAX, /* utc = */ false, /* isdst = */ -1, /* gmtoff = */ 0, ret);
 }
 
 static const char* extract_multiplier(const char *p, usec_t *ret) {

diff --git a/src/test/test-time-util.c b/src/test/test-time-util.c
index 236a0ccbc410b0039a3b1d2ddbe52d9f1fbbac32..7569f89ca0d6b3ee2e7f17165d52343825d5688f 100644 (file)
--- a/src/test/test-time-util.c
+++ b/src/test/test-time-util.c
@@ -403,35 +403,33 @@ TEST(format_timestamp) {
 }
 
 static void test_format_timestamp_impl(usec_t x) {
-        bool success, override;
-        const char *xx, *yy;
-        usec_t y, x_sec, y_sec;
-
-        xx = FORMAT_TIMESTAMP(x);
+        const char *xx = FORMAT_TIMESTAMP(x);
         ASSERT_NOT_NULL(xx);
+
+        usec_t y;
         ASSERT_OK(parse_timestamp(xx, &y));
-        yy = FORMAT_TIMESTAMP(y);
+        const char *yy = FORMAT_TIMESTAMP(y);
         ASSERT_NOT_NULL(yy);
 
-        x_sec = x / USEC_PER_SEC;
-        y_sec = y / USEC_PER_SEC;
-        success = (x_sec == y_sec) && streq(xx, yy);
-        /* Workaround for https://github.com/systemd/systemd/issues/28472
-         * and https://github.com/systemd/systemd/pull/35471. */
-        override = !success &&
-                   (STRPTR_IN_SET(tzname[0], "CAT", "EAT", "WET") ||
-                    STRPTR_IN_SET(tzname[1], "CAT", "EAT", "WET")) &&
-                   (x_sec > y_sec ? x_sec - y_sec : y_sec - x_sec) == 3600; /* 1 hour, ignore fractional second */
-        log_full(success ? LOG_DEBUG : override ? LOG_WARNING : LOG_ERR,
+        usec_t x_sec = x / USEC_PER_SEC;
+        usec_t y_sec = y / USEC_PER_SEC;
+
+        if (x_sec == y_sec && streq(xx, yy))
+                return; /* Yay!*/
+
+        /* When the timezone is built with rearguard being enabled (e.g. old Ubuntu and RHEL), the following
+         * timezone may provide time shifted 1 hour from the original. See
+         * https://github.com/systemd/systemd/issues/28472 and https://github.com/systemd/systemd/pull/35471 */
+        bool ignore =
+                streq_ptr(getenv("TZ"), ":Africa/Windhoek") &&
+                (x_sec > y_sec ? x_sec - y_sec : y_sec - x_sec) == 3600;
+
+        log_full(ignore ? LOG_WARNING : LOG_ERR,
                  "@" USEC_FMT " → %s → @" USEC_FMT " → %s%s",
                  x, xx, y, yy,
-                 override ? ", ignoring." : "");
-        if (!override) {
-                if (!success)
-                        log_warning("tzname[0]=\"%s\", tzname[1]=\"%s\"", tzname[0], tzname[1]);
-                ASSERT_EQ(x_sec, y_sec);
-                ASSERT_STREQ(xx, yy);
-        }
+                 ignore ? ", ignoring." : "");
+
+        ASSERT_TRUE(ignore);
 }
 
 static void test_format_timestamp_loop(void) {