From: dongshengyuan <545258830@qq.com> Date: Wed, 1 Jul 2026 06:57:33 +0000 (+0800) Subject: calendarspec: warn on weekday/date conflict in systemd-analyze and systemd-run X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=21de611f695b25d9fac59c96455fec9690b87b59;p=thirdparty%2Fsystemd.git calendarspec: warn on weekday/date conflict in systemd-analyze and systemd-run When a fixed date (e.g. 2027-01-01) is paired with a weekday constraint (e.g. Thu) that does not match, the timer silently never elapses. Add calendar_spec_from_string_full(..., warn_on_weekday_mismatch) so user-facing tools can opt in to a log_warning() at parse time: - systemd-analyze calendar: uses _full(true) - systemd-run --on-calendar: uses _full(true) - .timer OnCalendar=: uses log_syntax() with file/line context Add test_calendar_spec_weekday_conflict(): forks a child with stderr captured in a memfd via pidref_safe_fork_full(), verifies the warning is emitted for conflicting specs and suppressed for valid ones. Fixes: #40350 Signed-off-by: dongshengyuan --- diff --git a/src/analyze/analyze-calendar.c b/src/analyze/analyze-calendar.c index a9fb1e897e6..f6d434de36f 100644 --- a/src/analyze/analyze-calendar.c +++ b/src/analyze/analyze-calendar.c @@ -19,7 +19,7 @@ static int test_calendar_one(usec_t n, const char *p) { TableCell *cell; int r; - r = calendar_spec_from_string(p, &spec); + r = calendar_spec_from_string_full(p, &spec, /* warn_on_weekday_mismatch= */ true); if (r < 0) { log_error_errno(r, "Failed to parse calendar specification '%s': %m", p); time_parsing_hint(p, /* calendar= */ false, /* timestamp= */ true, /* timespan= */ true); diff --git a/src/basic/time-util.c b/src/basic/time-util.c index 93173c4d917..44c53067411 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -321,24 +321,26 @@ struct timeval *timeval_store(struct timeval *tv, usec_t u) { return tv; } +/* Returns the abbreviated English weekday name for wd in Mon=0 … Sun=6 order. + * We use non-localized (English) form so that timestamps can be parsed with + * parse_timestamp() and always read the same regardless of locale. */ +static const char *const weekday_table[_WEEKDAY_MAX] = { + [WEEKDAY_MON] = "Mon", + [WEEKDAY_TUE] = "Tue", + [WEEKDAY_WED] = "Wed", + [WEEKDAY_THU] = "Thu", + [WEEKDAY_FRI] = "Fri", + [WEEKDAY_SAT] = "Sat", + [WEEKDAY_SUN] = "Sun", +}; + +DEFINE_STRING_TABLE_LOOKUP_TO_STRING(weekday, int); + char* format_timestamp_style( char *buf, size_t l, usec_t t, TimestampStyle style) { - - /* The weekdays in non-localized (English) form. We use this instead of the localized form, so that - * our generated timestamps may be parsed with parse_timestamp(), and always read the same. */ - static const char * const weekdays[] = { - [0] = "Sun", - [1] = "Mon", - [2] = "Tue", - [3] = "Wed", - [4] = "Thu", - [5] = "Fri", - [6] = "Sat", - }; - struct tm tm; bool utc, us; size_t n; @@ -387,8 +389,9 @@ char* format_timestamp_style( return NULL; /* Start with the week day */ - assert((size_t) tm.tm_wday < ELEMENTSOF(weekdays)); - memcpy(buf, weekdays[tm.tm_wday], 4); + const char *weekday = weekday_to_string(tm.tm_wday == 0 ? 6 : tm.tm_wday - 1); + assert(weekday); + memcpy(buf, weekday, 4); if (style == TIMESTAMP_DATE) { /* Special format string if only date should be shown. */ diff --git a/src/basic/time-util.h b/src/basic/time-util.h index fb0aab3ff5c..12004497289 100644 --- a/src/basic/time-util.h +++ b/src/basic/time-util.h @@ -56,6 +56,18 @@ typedef enum TimestampStyle { #define USEC_PER_YEAR ((usec_t) (31557600ULL*USEC_PER_SEC)) #define NSEC_PER_YEAR ((nsec_t) (31557600ULL*NSEC_PER_SEC)) +enum { + WEEKDAY_MON, + WEEKDAY_TUE, + WEEKDAY_WED, + WEEKDAY_THU, + WEEKDAY_FRI, + WEEKDAY_SAT, + WEEKDAY_SUN, + _WEEKDAY_MAX, + _WEEKDAY_INVALID = -EINVAL, +}; + /* We assume a maximum timezone length of 6. TZNAME_MAX is not defined on Linux, but glibc internally initializes this * to 6. Let's rely on that. */ #define FORMAT_TIMESTAMP_MAX (3U+1U+10U+1U+8U+1U+6U+1U+6U+1U) @@ -125,6 +137,10 @@ char* format_timestamp_style(char *buf, size_t l, usec_t t, TimestampStyle style char* format_timestamp_relative_full(char *buf, size_t l, usec_t t, clockid_t clock, bool implicit_left) _warn_unused_result_; char* format_timespan(char *buf, size_t l, usec_t t, usec_t accuracy) _warn_unused_result_; +/* Returns the abbreviated English weekday name for wd in Mon=0 … Sun=6 order + * (matching systemd's weekdays_bits layout). */ +const char* weekday_to_string(int i); + _warn_unused_result_ static inline char* format_timestamp_relative(char *buf, size_t l, usec_t t) { return format_timestamp_relative_full(buf, l, t, CLOCK_REALTIME, /* implicit_left= */ false); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index c9e270a6682..bf17e2df46f 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2082,6 +2082,14 @@ int config_parse_timer( log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse calendar specification, ignoring: %s", k); return 0; } + + int wday; + if (calendar_spec_weekday_conflicts(c, &wday)) + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Weekday constraint does not match the fixed date %04d-%02d-%02d " + "(which is a %s), so this timer will never elapse.", + c->year->start, c->month->start, c->day->start, + weekday_to_string(wday)); } else { r = parse_sec(k, &usec); if (r < 0) { diff --git a/src/run/run.c b/src/run/run.c index 1fca68c76f8..068e0cc29fe 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -552,19 +552,24 @@ static int parse_argv(int argc, char *argv[]) { OPTION_LONG("on-calendar", "SPEC", "Realtime timer"): { _cleanup_(calendar_spec_freep) CalendarSpec *cs = NULL; - r = calendar_spec_from_string(opts.arg, &cs); + r = calendar_spec_from_string_full(opts.arg, &cs, /* warn_on_weekday_mismatch= */ true); if (r < 0) return log_error_errno(r, "Failed to parse calendar event specification: %m"); /* Let's make sure the given calendar event is not in the past */ r = calendar_spec_next_usec(cs, now(CLOCK_REALTIME), NULL); - if (r == -ENOENT) - /* The calendar event is in the past — let's warn about this, but install it - * anyway as is. The service manager will trigger the service right away. - * Moreover, the server side might have a different clock or timezone than we - * do, hence it should decide when or whether to run something. */ - log_warning("Specified calendar expression is in the past, proceeding anyway."); - else if (r < 0) + if (r == -ENOENT) { + /* The calendar event might be in the past, so let's warn about this, but + * install it anyway as is. The service manager will trigger the service + * right away. Moreover, the server side might have a different clock or + * timezone than we do, hence it should decide when or whether to run + * something. + * + * However, a mismatching weekday for a fixed date also results in -ENOENT, + * and was already warned about when parsing. */ + if (!calendar_spec_weekday_conflicts(cs, NULL)) + log_warning("Specified calendar expression is in the past, proceeding anyway."); + } else if (r < 0) return log_error_errno(r, "Failed to calculate next time calendar expression elapses: %m"); r = add_timer_property("OnCalendar", opts.arg); diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index 225d811d192..6a56438c1c7 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -15,7 +15,7 @@ #include "strv.h" #include "time-util.h" -#define BITS_WEEKDAYS 127 +#define BITS_WEEKDAYS ((1 << _WEEKDAY_MAX) - 1) #define MIN_YEAR 1970 #define MAX_YEAR 2199 @@ -237,16 +237,6 @@ _pure_ bool calendar_spec_valid(CalendarSpec *c) { } static void format_weekdays(FILE *f, const CalendarSpec *c) { - static const char *const days[] = { - "Mon", - "Tue", - "Wed", - "Thu", - "Fri", - "Sat", - "Sun", - }; - int l, x; bool need_comma = false; @@ -254,7 +244,7 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) { assert(c); assert(c->weekdays_bits > 0 && c->weekdays_bits <= BITS_WEEKDAYS); - for (x = 0, l = -1; x < (int) ELEMENTSOF(days); x++) { + for (x = 0, l = -1; x < _WEEKDAY_MAX; x++) { if (c->weekdays_bits & (1 << x)) { @@ -264,7 +254,7 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) { else need_comma = true; - fputs(days[x], f); + fputs(weekday_to_string(x), f); l = x; } @@ -272,7 +262,7 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) { if (x > l + 1) { fputs(x > l + 2 ? ".." : ",", f); - fputs(days[x-1], f); + fputs(weekday_to_string(x-1), f); } l = -1; @@ -281,7 +271,7 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) { if (l >= 0 && x > l + 1) { fputs(x > l + 2 ? ".." : ",", f); - fputs(days[x-1], f); + fputs(weekday_to_string(x-1), f); } } @@ -877,7 +867,7 @@ finish: return 0; } -int calendar_spec_from_string(const char *p, CalendarSpec **ret) { +int calendar_spec_from_string_full(const char *p, CalendarSpec **ret, bool warn_on_weekday_mismatch) { const char *utc; _cleanup_(calendar_spec_freep) CalendarSpec *c = NULL; _cleanup_free_ char *p_tmp = NULL; @@ -1098,6 +1088,15 @@ int calendar_spec_from_string(const char *p, CalendarSpec **ret) { if (!calendar_spec_valid(c)) return -EINVAL; + if (warn_on_weekday_mismatch) { + int wday; + if (calendar_spec_weekday_conflicts(c, &wday)) + log_warning("Weekday constraint does not match the fixed date %04d-%02d-%02d " + "(which is a %s), so this timer will never elapse.", + c->year->start, c->month->start, c->day->start, + weekday_to_string(wday)); + } + if (ret) *ret = TAKE_PTR(c); return 0; @@ -1228,6 +1227,7 @@ static int tm_within_bounds(struct tm *tm, bool utc) { static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) { struct tm t; + usec_t usec; int k; assert(tm); @@ -1236,13 +1236,64 @@ static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) { return true; t = *tm; - if (mktime_or_timegm_usec(&t, utc, /* ret= */ NULL) < 0) + if (mktime_or_timegm_usec(&t, utc, &usec) < 0) + return false; + if (localtime_or_gmtime_usec(usec, utc, &t) < 0) return false; k = t.tm_wday == 0 ? 6 : t.tm_wday - 1; return (weekdays_bits & (1 << k)); } +static bool component_is_single_fixed(const CalendarComponent *c) { + /* Returns true if the component is a single fixed value: no range, no repeat, no alternatives. */ + return c && !c->next && c->repeat == 0 && (c->stop < 0 || c->stop == c->start); +} + +bool calendar_spec_weekday_conflicts(const CalendarSpec *spec, int *ret_actual_wday) { + assert(spec); + + if (spec->weekdays_bits <= 0 || spec->weekdays_bits >= BITS_WEEKDAYS) + return false; + + /* Skip when end_of_month (~) is used: day->start holds an offset, not an + * absolute day-of-month, so the real firing date cannot be determined here. */ + if (spec->end_of_month) + return false; + + /* Only detectable when year, month and day are each a single fixed value */ + if (!component_is_single_fixed(spec->year) || + !component_is_single_fixed(spec->month) || + !component_is_single_fixed(spec->day)) + return false; + + /* CalendarSpec stores month as 1-12; struct tm uses 0-11 */ + struct tm tm = { + .tm_year = spec->year->start - 1900, + .tm_mon = spec->month->start - 1, + .tm_mday = spec->day->start, + }; + struct tm orig = tm; + + /* Compute weekday in UTC to avoid TZ/DST skew; reject dates that would be + * silently normalised (e.g. 2027-02-31 → 2027-03-03). + * + * Do not rely on timegm() to fill tm_wday, since musl does not do that. */ + usec_t usec; + if (mktime_or_timegm_usec(&tm, /* utc= */ true, &usec) < 0 || + localtime_or_gmtime_usec(usec, /* utc= */ true, &tm) < 0 || + tm.tm_year != orig.tm_year || tm.tm_mon != orig.tm_mon || + tm.tm_mday != orig.tm_mday) + return false; + + if (matches_weekday(spec->weekdays_bits, &tm, /* utc= */ true)) + return false; /* no conflict */ + + if (ret_actual_wday) + *ret_actual_wday = tm.tm_wday == 0 ? 6 : tm.tm_wday - 1; + return true; +} + static int tm_compare(const struct tm *t1, const struct tm *t2) { int r; diff --git a/src/shared/calendarspec.h b/src/shared/calendarspec.h index ce76a7c1c62..339dda30abe 100644 --- a/src/shared/calendarspec.h +++ b/src/shared/calendarspec.h @@ -35,8 +35,13 @@ CalendarSpec* calendar_spec_free(CalendarSpec *c); bool calendar_spec_valid(CalendarSpec *spec); int calendar_spec_to_string(const CalendarSpec *spec, char **ret); -int calendar_spec_from_string(const char *p, CalendarSpec **ret); +int calendar_spec_from_string_full(const char *p, CalendarSpec **ret, bool warn_on_weekday_mismatch); + +static inline int calendar_spec_from_string(const char *p, CalendarSpec **ret) { + return calendar_spec_from_string_full(p, ret, /* warn_on_weekday_mismatch= */ false); +} int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next); +bool calendar_spec_weekday_conflicts(const CalendarSpec *spec, int *ret_actual_wday); DEFINE_TRIVIAL_CLEANUP_FUNC(CalendarSpec*, calendar_spec_free); diff --git a/src/test/test-calendarspec.c b/src/test/test-calendarspec.c index 6c901b1893c..0d517185611 100644 --- a/src/test/test-calendarspec.c +++ b/src/test/test-calendarspec.c @@ -7,6 +7,11 @@ #include "calendarspec.h" #include "env-util.h" #include "errno-util.h" +#include "fd-util.h" +#include "io-util.h" +#include "log.h" +#include "memfd-util.h" +#include "process-util.h" #include "string-util.h" #include "tests.h" #include "time-util.h" @@ -256,6 +261,76 @@ TEST(calendar_spec_from_string) { assert_se(calendar_spec_from_string("*:4,30:*\n", &c) == -EINVAL); } +/* Fork a child with stderr redirected to a memfd, invoke calendar_spec_from_string_full() + * with warn=true, wait, then return the memfd rewound to 0. */ +static int run_in_child_capture_stderr(const char *spec) { + _cleanup_close_ int memfd = memfd_new("calendarspec-warn-test"); + assert_se(memfd >= 0); + + int r = pidref_safe_fork_full( + "(calendarspec-warn)", + (const int[3]) { -1, -1, memfd }, + NULL, 0, + FORK_WAIT|FORK_LOG|FORK_REARRANGE_STDIO, + NULL); + assert_se(r >= 0); + + if (r == 0) { + /* log.c may have inherited a console fd that no longer points at stderr. */ + log_close(); + log_set_target_and_open(LOG_TARGET_CONSOLE); + log_set_max_level(LOG_WARNING); + + _cleanup_(calendar_spec_freep) CalendarSpec *c = NULL; + (void) calendar_spec_from_string_full(spec, &c, /* warn_on_weekday_mismatch= */ true); + /* Write PARSE_OK to stderr as a positive anchor. */ + fputs("PARSE_OK\n", stderr); + _exit(0); + } + + assert_se(lseek(memfd, 0, SEEK_SET) == 0); + return TAKE_FD(memfd); +} + +TEST(calendar_spec_weekday_conflict) { + char buf[4096]; + ssize_t n; + + /* Case 1: conflicting weekday — warning must be emitted */ + { + _cleanup_close_ int fd = run_in_child_capture_stderr("Thu 2027-01-01"); + n = loop_read(fd, buf, sizeof(buf) - 1, /* do_poll= */ false); + assert_se(n > 0); + buf[n] = '\0'; + /* Check date, weekday text and warning phrase are present. */ + assert_se(strstr(buf, "2027-01-01")); + assert_se(strstr(buf, "which is a Fri")); + assert_se(strstr(buf, "will never elapse")); + } + + /* Case 2: correct weekday — no warning */ + { + _cleanup_close_ int fd = run_in_child_capture_stderr("Fri 2027-01-01"); + n = loop_read(fd, buf, sizeof(buf) - 1, /* do_poll= */ false); + /* Ensure child wrote PARSE_OK. */ + assert_se(n >= 0); + buf[MAX(n, (ssize_t)0)] = '\0'; + assert_se(strstr(buf, "PARSE_OK")); + assert_se(!strstr(buf, "will never elapse")); + } + + /* Case 3: wildcard date — no static conflict, no warning */ + { + _cleanup_close_ int fd = run_in_child_capture_stderr("Thu *-*-*"); + n = loop_read(fd, buf, sizeof(buf) - 1, /* do_poll= */ false); + /* Ensure child wrote PARSE_OK. */ + assert_se(n >= 0); + buf[MAX(n, (ssize_t)0)] = '\0'; + assert_se(strstr(buf, "PARSE_OK")); + assert_se(!strstr(buf, "will never elapse")); + } +} + static int intro(void) { /* Tests have hard-coded results that do not expect a specific timezone to be set by the caller */ ASSERT_OK_ERRNO(unsetenv("TZ"));