From a5b357c8a666b0e7acb03f41f43682ea3457d345 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Fri, 7 Nov 2025 16:35:51 -0600 Subject: [PATCH 1/9] Refactor: libcrmcommon: Drop pcmk__time_hr_new() The only callers were a fuzzer line (which was only there to test this function) and a unit test of a different function. We can just do the assignment inside the unit test. This leaves a little bit of duplication with time_to_hr(), but the goal is to replace most of our iso8601 library with something external (see upstream task T432). So I'm not concerned about duplication in one place. This does requires exposing struct crm_time_s internally, so that we can reference its members in the unit test. Alternatively we could create getter functions for the members, but this is easier. Signed-off-by: Reid Wahl --- include/crm/common/iso8601_internal.h | 22 +++++++++++++++- lib/common/fuzzers/iso8601_fuzzer.c | 5 +--- lib/common/iso8601.c | 26 ------------------- .../tests/iso8601/pcmk__time_format_hr_test.c | 26 +++++++++++++------ 4 files changed, 40 insertions(+), 39 deletions(-) diff --git a/include/crm/common/iso8601_internal.h b/include/crm/common/iso8601_internal.h index 7226e8b1662..8db1dc52b5f 100644 --- a/include/crm/common/iso8601_internal.h +++ b/include/crm/common/iso8601_internal.h @@ -23,7 +23,6 @@ extern "C" { typedef struct pcmk__time_us pcmk__time_hr_t; pcmk__time_hr_t *pcmk__time_hr_now(time_t *epoch); -pcmk__time_hr_t *pcmk__time_hr_new(const char *date_time); void pcmk__time_hr_free(pcmk__time_hr_t *hr_dt); char *pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt); char *pcmk__epoch2str(const time_t *source, uint32_t flags); @@ -31,6 +30,27 @@ char *pcmk__timespec2str(const struct timespec *ts, uint32_t flags); const char *pcmk__readable_interval(guint interval_ms); crm_time_t *pcmk__copy_timet(time_t source); +// A date/time or duration +struct crm_time_s { + // Calendar year (date/time) or number of years (duration) + int years; + + // Number of months (duration only) + int months; + + // Ordinal day of year (date/time) or number of days (duration) + int days; + + // Seconds of day (date/time) or number of seconds (duration) + int seconds; + + // Seconds offset from UTC (date/time only) + int offset; + + // True if duration + bool duration; +}; + struct pcmk__time_us { int years; int months; /* Only for durations */ diff --git a/lib/common/fuzzers/iso8601_fuzzer.c b/lib/common/fuzzers/iso8601_fuzzer.c index 0e151c6dbd8..d5cca7a9635 100644 --- a/lib/common/fuzzers/iso8601_fuzzer.c +++ b/lib/common/fuzzers/iso8601_fuzzer.c @@ -1,5 +1,5 @@ /* - * Copyright 2024 the Pacemaker project contributors + * Copyright 2024-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -33,9 +33,6 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) period = crm_time_parse_period(ns); crm_time_free_period(period); - now = pcmk__time_hr_new(ns); - pcmk__time_hr_free(now); - now = pcmk__time_hr_now(&epoch); result = pcmk__time_format_hr(ns, now); pcmk__time_hr_free(now); diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index 0b419e69c86..39e07697b16 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -69,16 +69,6 @@ ((QB_ABS(usec) < QB_TIME_US_IN_SEC) \ && (((sec) == 0) || ((usec) == 0) || (((sec) < 0) == ((usec) < 0)))) -// A date/time or duration -struct crm_time_s { - int years; // Calendar year (date/time) or number of years (duration) - int months; // Number of months (duration only) - int days; // Ordinal day of year (date/time) or number of days (duration) - int seconds; // Seconds of day (date/time) or number of seconds (duration) - int offset; // Seconds offset from UTC (date/time only) - bool duration; // True if duration -}; - static crm_time_t *parse_date(const char *date_str); static crm_time_t * @@ -1962,22 +1952,6 @@ pcmk__time_hr_now(time_t *epoch) return hr; } -pcmk__time_hr_t * -pcmk__time_hr_new(const char *date_time) -{ - pcmk__time_hr_t *hr_dt = NULL; - - if (date_time == NULL) { - hr_dt = pcmk__time_hr_now(NULL); - } else { - crm_time_t *dt = parse_date(date_time); - - hr_dt = time_to_hr(dt); - crm_time_free(dt); - } - return hr_dt; -} - void pcmk__time_hr_free(pcmk__time_hr_t * hr_dt) { diff --git a/lib/common/tests/iso8601/pcmk__time_format_hr_test.c b/lib/common/tests/iso8601/pcmk__time_format_hr_test.c index 3b9ad79d362..c6a37443aaf 100644 --- a/lib/common/tests/iso8601/pcmk__time_format_hr_test.c +++ b/lib/common/tests/iso8601/pcmk__time_format_hr_test.c @@ -14,16 +14,17 @@ #include #include -#define DATE_S "2024-06-02" +#define YEAR_S "2024" +#define MONTH_S "06" +#define DAY_S "02" +#define DATE_S YEAR_S "-" MONTH_S "-" DAY_S #define HOUR_S "03" #define MINUTE_S "04" #define SECOND_S "05" #define TIME_S HOUR_S ":" MINUTE_S ":" SECOND_S -#define OFFSET_S "+00:00" - -#define TEST_TIME pcmk__time_hr_new(DATE_S " " TIME_S " " OFFSET_S) +#define DATE_TIME_S DATE_S " " TIME_S /*! * \internal @@ -42,12 +43,21 @@ static void assert_hr_format(const char *format, const char *expected, const char *alternate, int usec) { - pcmk__time_hr_t *hr = TEST_TIME; + crm_time_t *dt = crm_time_new(DATE_TIME_S); + pcmk__time_hr_t hr_dt = { 0, }; char *result = NULL; - hr->useconds = usec; - result = pcmk__time_format_hr(format, hr); - pcmk__time_hr_free(hr); + assert_non_null(dt); + + hr_dt.years = dt->years; + hr_dt.months = dt->months; + hr_dt.days = dt->days; + hr_dt.seconds = dt->seconds; + hr_dt.offset = dt->offset; + hr_dt.duration = dt->duration; + hr_dt.useconds = usec; + + result = pcmk__time_format_hr(format, &hr_dt); if (expected == NULL) { assert_null(result); From 451ca6c3e3330871b12f8ff88f5f5c05ab55afc6 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 8 Nov 2025 11:44:11 -0600 Subject: [PATCH 2/9] Refactor: libcrmcommon: Drop struct pcmk__time_us Signed-off-by: Reid Wahl --- include/crm/common/iso8601_internal.h | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/include/crm/common/iso8601_internal.h b/include/crm/common/iso8601_internal.h index 8db1dc52b5f..2a40d59eda8 100644 --- a/include/crm/common/iso8601_internal.h +++ b/include/crm/common/iso8601_internal.h @@ -20,7 +20,15 @@ extern "C" { #endif -typedef struct pcmk__time_us pcmk__time_hr_t; +typedef struct { + int years; + int months; // Only for durations + int days; + int seconds; + int offset; // In seconds + bool duration; + int useconds; +} pcmk__time_hr_t; pcmk__time_hr_t *pcmk__time_hr_now(time_t *epoch); void pcmk__time_hr_free(pcmk__time_hr_t *hr_dt); @@ -51,16 +59,6 @@ struct crm_time_s { bool duration; }; -struct pcmk__time_us { - int years; - int months; /* Only for durations */ - int days; - int seconds; - int offset; /* Seconds */ - bool duration; - int useconds; -}; - #ifdef __cplusplus } #endif From c8931ad9913b43d2aca9b64caee6259d1188eea2 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 8 Nov 2025 11:52:28 -0600 Subject: [PATCH 3/9] Refactor: libcrmcommon: Drop time_to_hr() This aims to address a false positive issue found by a fuzzer. See: https://github.com/ClusterLabs/pacemaker/pull/3985 https://issues.oss-fuzz.com/issues/456526118 (likely will get "Access Denied") Signed-off-by: Reid Wahl --- lib/common/iso8601.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index 39e07697b16..bc33ba7e5d5 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -1910,23 +1910,6 @@ crm_time_add_years(crm_time_t * a_time, int extra) * crm_time_t, pcmk__time_hr_t, and struct timespec (in lrmd_cmd_t). */ -static pcmk__time_hr_t * -time_to_hr(const crm_time_t *dt) -{ - pcmk__time_hr_t *hr_dt = NULL; - - pcmk__assert(dt != NULL); - - hr_dt = pcmk__assert_alloc(1, sizeof(pcmk__time_hr_t)); - hr_dt->years = dt->years; - hr_dt->months = dt->months; - hr_dt->days = dt->days; - hr_dt->seconds = dt->seconds; - hr_dt->offset = dt->offset; - hr_dt->duration = dt->duration; - return hr_dt; -} - /*! * \internal * \brief Return the current time as a high-resolution time @@ -1938,16 +1921,24 @@ time_to_hr(const crm_time_t *dt) pcmk__time_hr_t * pcmk__time_hr_now(time_t *epoch) { - struct timespec tv; - crm_time_t dt; - pcmk__time_hr_t *hr; + struct timespec tv = { 0, }; + crm_time_t dt = { 0, }; + pcmk__time_hr_t *hr = pcmk__assert_alloc(1, sizeof(pcmk__time_hr_t)); qb_util_timespec_from_epoch_get(&tv); + if (epoch != NULL) { *epoch = tv.tv_sec; } + crm_time_set_timet(&dt, &(tv.tv_sec)); - hr = time_to_hr(&dt); + + hr->years = dt.years; + hr->months = dt.months; + hr->days = dt.days; + hr->seconds = dt.seconds; + hr->offset = dt.offset; + hr->duration = dt.duration; hr->useconds = tv.tv_nsec / QB_TIME_NS_IN_USEC; return hr; } From 2ce5c8433990243f7fcbe842e7582f298212382e Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 8 Nov 2025 12:01:11 -0600 Subject: [PATCH 4/9] Refactor: libcrmcommon: Drop pcmk__time_hr_free() Signed-off-by: Reid Wahl --- include/crm/common/iso8601_internal.h | 1 - lib/common/fuzzers/iso8601_fuzzer.c | 2 +- lib/common/iso8601.c | 8 ++------ 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/include/crm/common/iso8601_internal.h b/include/crm/common/iso8601_internal.h index 2a40d59eda8..638a9e59de4 100644 --- a/include/crm/common/iso8601_internal.h +++ b/include/crm/common/iso8601_internal.h @@ -31,7 +31,6 @@ typedef struct { } pcmk__time_hr_t; pcmk__time_hr_t *pcmk__time_hr_now(time_t *epoch); -void pcmk__time_hr_free(pcmk__time_hr_t *hr_dt); char *pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt); char *pcmk__epoch2str(const time_t *source, uint32_t flags); char *pcmk__timespec2str(const struct timespec *ts, uint32_t flags); diff --git a/lib/common/fuzzers/iso8601_fuzzer.c b/lib/common/fuzzers/iso8601_fuzzer.c index d5cca7a9635..90f5cd0226c 100644 --- a/lib/common/fuzzers/iso8601_fuzzer.c +++ b/lib/common/fuzzers/iso8601_fuzzer.c @@ -35,7 +35,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) now = pcmk__time_hr_now(&epoch); result = pcmk__time_format_hr(ns, now); - pcmk__time_hr_free(now); + free(now); free(result); free(ns); diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index bc33ba7e5d5..b91db62e757 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -1917,6 +1917,8 @@ crm_time_add_years(crm_time_t * a_time, int extra) * \param[out] epoch If not NULL, this will be set to seconds since epoch * * \return Newly allocated high-resolution time set to the current time + * + * \note The caller is responsible for freeing the return value using \c free(). */ pcmk__time_hr_t * pcmk__time_hr_now(time_t *epoch) @@ -1943,12 +1945,6 @@ pcmk__time_hr_now(time_t *epoch) return hr; } -void -pcmk__time_hr_free(pcmk__time_hr_t * hr_dt) -{ - free(hr_dt); -} - static void ha_get_tm_time(struct tm *target, const pcmk__time_hr_t *source) { From cc1be9d04dbaaff19c6799c05d7be30fb1e05109 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 8 Nov 2025 12:19:49 -0600 Subject: [PATCH 5/9] Refactor: liblrmd: Remove some nesting from exec_alert_list() And implement a few other best practices, like bool instead of gboolean, limiting scope, and using const. Replace three temporary string variables (timestamp, timestamp_epoch, and timestamp_usec) with one (str). Use the fact that the now variable is guaranteed non-NULL by the time we reach the place where we set the three timestamp params, to remove some nesting. Signed-off-by: Reid Wahl --- lib/common/iso8601.c | 1 + lib/lrmd/lrmd_alerts.c | 61 +++++++++++++++++++----------------------- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index b91db62e757..6218d14e3ba 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -1918,6 +1918,7 @@ crm_time_add_years(crm_time_t * a_time, int extra) * * \return Newly allocated high-resolution time set to the current time * + * \note The return value is guaranteed not to be \c NULL. * \note The caller is responsible for freeing the return value using \c free(). */ pcmk__time_hr_t * diff --git a/lib/lrmd/lrmd_alerts.c b/lib/lrmd/lrmd_alerts.c index 79798425445..309ffd65784 100644 --- a/lib/lrmd/lrmd_alerts.c +++ b/lib/lrmd/lrmd_alerts.c @@ -123,7 +123,8 @@ exec_alert_list(lrmd_t *lrmd, const GList *alert_list, enum pcmk__alert_flags kind, const char *attr_name, lrmd_key_value_t *params) { - bool any_success = FALSE, any_failure = FALSE; + bool any_success = false; + bool any_failure = false; const char *kind_s = pcmk__alert_flag2text(kind); pcmk__time_hr_t *now = NULL; time_t epoch = 0; @@ -132,12 +133,11 @@ exec_alert_list(lrmd_t *lrmd, const GList *alert_list, params = alert_key2param(params, PCMK__alert_key_version, PACEMAKER_VERSION); - for (const GList *iter = alert_list; - iter != NULL; iter = g_list_next(iter)) { - const pcmk__alert_t *entry = (pcmk__alert_t *) (iter->data); + for (const GList *iter = alert_list; iter != NULL; iter = iter->next) { + const pcmk__alert_t *entry = iter->data; lrmd_key_value_t *copy_params = NULL; - lrmd_key_value_t *head = NULL; - int rc; + char *str = NULL; + int rc = pcmk_ok; if (!pcmk__is_set(entry->flags, kind)) { crm_trace("Filtering unwanted %s alert to %s via %s", @@ -160,36 +160,33 @@ exec_alert_list(lrmd_t *lrmd, const GList *alert_list, kind_s, entry->id, entry->recipient); /* Make a copy of the parameters, because each alert will be unique */ - for (head = params; head != NULL; head = head->next) { - copy_params = lrmd_key_value_add(copy_params, head->key, head->value); + for (const lrmd_key_value_t *param = params; param != NULL; + param = param->next) { + + copy_params = lrmd_key_value_add(copy_params, param->key, + param->value); } copy_params = alert_key2param(copy_params, PCMK__alert_key_recipient, entry->recipient); - if (now) { - char *timestamp = pcmk__time_format_hr(entry->tstamp_format, now); - char *timestamp_epoch = pcmk__assert_asprintf("%lld", - (long long) epoch); - char *timestamp_usec = pcmk__assert_asprintf("%06d", now->useconds); - - if (timestamp) { - copy_params = alert_key2param(copy_params, - PCMK__alert_key_timestamp, - timestamp); - free(timestamp); - } - - copy_params = alert_key2param(copy_params, - PCMK__alert_key_timestamp_epoch, - timestamp_epoch); + str = pcmk__time_format_hr(entry->tstamp_format, now); + if (str != NULL) { copy_params = alert_key2param(copy_params, - PCMK__alert_key_timestamp_usec, - timestamp_usec); - free(timestamp_epoch); - free(timestamp_usec); + PCMK__alert_key_timestamp, str); + free(str); } + str = pcmk__assert_asprintf("%lld", (long long) epoch); + copy_params = alert_key2param(copy_params, + PCMK__alert_key_timestamp_epoch, str); + free(str); + + str = pcmk__assert_asprintf("%06d", now->useconds); + copy_params = alert_key2param(copy_params, + PCMK__alert_key_timestamp_usec, str); + free(str); + copy_params = alert_envvar2params(copy_params, entry); rc = lrmd->cmds->exec_alert(lrmd, entry->id, entry->path, @@ -197,15 +194,13 @@ exec_alert_list(lrmd_t *lrmd, const GList *alert_list, if (rc < 0) { crm_err("Could not execute alert %s: %s " QB_XS " rc=%d", entry->id, pcmk_strerror(rc), rc); - any_failure = TRUE; + any_failure = true; } else { - any_success = TRUE; + any_success = true; } } - if (now) { - free(now); - } + free(now); if (any_failure) { return (any_success? -1 : -2); From 85337eb9a08637b7210399399f2f6b73cf5a46b4 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 8 Nov 2025 13:02:08 -0600 Subject: [PATCH 6/9] Refactor: libcrmcommon: pcmk__time_format_hr() takes crm_time_t and int A pcmk__time_hr_t object is just a crm_time_t with an int appended for useconds. Instead of taking that object, having to construct it, etc., just take the crm_time_t and int. This removes the sole use of pcmk__time_hr_t, so that we can drop it. This makes sense to do, given that our goal is to replace most of our time library with something external (see T432). Signed-off-by: Reid Wahl --- include/crm/common/iso8601_internal.h | 2 +- lib/common/fuzzers/iso8601_fuzzer.c | 17 +++++++++------ lib/common/iso8601.c | 13 ++++++------ .../tests/iso8601/pcmk__time_format_hr_test.c | 17 +++++---------- lib/lrmd/lrmd_alerts.c | 21 ++++++++++--------- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/include/crm/common/iso8601_internal.h b/include/crm/common/iso8601_internal.h index 638a9e59de4..38e1c4cf358 100644 --- a/include/crm/common/iso8601_internal.h +++ b/include/crm/common/iso8601_internal.h @@ -31,7 +31,7 @@ typedef struct { } pcmk__time_hr_t; pcmk__time_hr_t *pcmk__time_hr_now(time_t *epoch); -char *pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt); +char *pcmk__time_format_hr(const char *format, const crm_time_t *dt, int usec); char *pcmk__epoch2str(const time_t *source, uint32_t flags); char *pcmk__timespec2str(const struct timespec *ts, uint32_t flags); const char *pcmk__readable_interval(guint interval_ms); diff --git a/lib/common/fuzzers/iso8601_fuzzer.c b/lib/common/fuzzers/iso8601_fuzzer.c index 90f5cd0226c..65a2d430cea 100644 --- a/lib/common/fuzzers/iso8601_fuzzer.c +++ b/lib/common/fuzzers/iso8601_fuzzer.c @@ -10,6 +10,9 @@ #include #include #include +#include // struct timespec + +#include // qb_util_timespec_from_epoch_get() #include #include @@ -18,11 +21,12 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { char *ns = NULL; - char *result = NULL; - time_t epoch = 0; - pcmk__time_hr_t *now = NULL; crm_time_period_t *period = NULL; + struct timespec tv = { 0, }; + crm_time_t now = { 0, }; + char *result = NULL; + // Ensure we have enough data. if (size < 10) { return -1; // Do not add input to testing corpus @@ -33,9 +37,10 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) period = crm_time_parse_period(ns); crm_time_free_period(period); - now = pcmk__time_hr_now(&epoch); - result = pcmk__time_format_hr(ns, now); - free(now); + qb_util_timespec_from_epoch_get(&tv); + crm_time_set_timet(&now, &(tv.tv_sec)); + result = pcmk__time_format_hr(ns, &now, + (int) (tv.tv_nsec / QB_TIME_NS_IN_USEC)); free(result); free(ns); diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index 6218d14e3ba..5ea487688a4 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -1947,7 +1947,7 @@ pcmk__time_hr_now(time_t *epoch) } static void -ha_get_tm_time(struct tm *target, const pcmk__time_hr_t *source) +ha_get_tm_time(struct tm *target, const crm_time_t *source) { *target = (struct tm) { .tm_year = source->years - 1900, @@ -2036,7 +2036,8 @@ get_g_date_time(const struct tm *tm, int offset) * \param[in] format Date/time format string compatible with * \c g_date_time_format(), with additional support for * \c "%N" for fractional seconds - * \param[in] hr_dt Time value to format + * \param[in] dt Time value to format (at seconds resolution) + * \param[in] usec Microseconds to add to \p dt when formatting * * \return Newly allocated string with formatted string, or \c NULL on error * @@ -2045,7 +2046,7 @@ get_g_date_time(const struct tm *tm, int offset) * in a future release. */ char * -pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt) +pcmk__time_format_hr(const char *format, const crm_time_t *dt, int usec) { int scanned_pos = 0; // How many characters of format have been parsed int printed_pos = 0; // How many characters of format have been processed @@ -2061,8 +2062,8 @@ pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt) buf = g_string_sized_new(128); - ha_get_tm_time(&tm, hr_dt); - gdt = get_g_date_time(&tm, hr_dt->offset); + ha_get_tm_time(&tm, dt); + gdt = get_g_date_time(&tm, dt->offset); if (gdt == NULL) { goto done; } @@ -2187,7 +2188,7 @@ pcmk__time_format_hr(const char *format, const pcmk__time_hr_t *hr_dt) * our microseconds value by 10^0 == 1, which is powers[6 - 1]. */ g_string_append_printf(buf, "%0*d", frac_digits, - hr_dt->useconds / powers[frac_digits - 1]); + usec / powers[frac_digits - 1]); } } diff --git a/lib/common/tests/iso8601/pcmk__time_format_hr_test.c b/lib/common/tests/iso8601/pcmk__time_format_hr_test.c index c6a37443aaf..8c27a868119 100644 --- a/lib/common/tests/iso8601/pcmk__time_format_hr_test.c +++ b/lib/common/tests/iso8601/pcmk__time_format_hr_test.c @@ -44,20 +44,11 @@ assert_hr_format(const char *format, const char *expected, const char *alternate, int usec) { crm_time_t *dt = crm_time_new(DATE_TIME_S); - pcmk__time_hr_t hr_dt = { 0, }; char *result = NULL; assert_non_null(dt); - hr_dt.years = dt->years; - hr_dt.months = dt->months; - hr_dt.days = dt->days; - hr_dt.seconds = dt->seconds; - hr_dt.offset = dt->offset; - hr_dt.duration = dt->duration; - hr_dt.useconds = usec; - - result = pcmk__time_format_hr(format, &hr_dt); + result = pcmk__time_format_hr(format, dt, usec); if (expected == NULL) { assert_null(result); @@ -79,8 +70,10 @@ assert_hr_format(const char *format, const char *expected, static void null_format(void **state) { - assert_null(pcmk__time_format_hr(NULL, NULL)); - assert_hr_format(NULL, NULL, NULL, 0); // for pcmk__time_format_hr(NULL, hr) + assert_null(pcmk__time_format_hr(NULL, NULL, 0)); + + // For pcmk__time_format_hr(NULL, dt, 0) + assert_hr_format(NULL, NULL, NULL, 0); } static void diff --git a/lib/lrmd/lrmd_alerts.c b/lib/lrmd/lrmd_alerts.c index 309ffd65784..392e099189f 100644 --- a/lib/lrmd/lrmd_alerts.c +++ b/lib/lrmd/lrmd_alerts.c @@ -126,8 +126,14 @@ exec_alert_list(lrmd_t *lrmd, const GList *alert_list, bool any_success = false; bool any_failure = false; const char *kind_s = pcmk__alert_flag2text(kind); - pcmk__time_hr_t *now = NULL; - time_t epoch = 0; + + struct timespec now_tv = { 0, }; + crm_time_t now_dt = { 0, }; + int now_usec = 0; + + qb_util_timespec_from_epoch_get(&now_tv); + crm_time_set_timet(&now_dt, &(now_tv.tv_sec)); + now_usec = now_tv.tv_nsec / QB_TIME_NS_IN_USEC; params = alert_key2param(params, PCMK__alert_key_kind, kind_s); params = alert_key2param(params, PCMK__alert_key_version, @@ -153,9 +159,6 @@ exec_alert_list(lrmd_t *lrmd, const GList *alert_list, continue; } - if (now == NULL) { - now = pcmk__time_hr_now(&epoch); - } crm_info("Sending %s alert via %s to %s", kind_s, entry->id, entry->recipient); @@ -170,19 +173,19 @@ exec_alert_list(lrmd_t *lrmd, const GList *alert_list, copy_params = alert_key2param(copy_params, PCMK__alert_key_recipient, entry->recipient); - str = pcmk__time_format_hr(entry->tstamp_format, now); + str = pcmk__time_format_hr(entry->tstamp_format, &now_dt, now_usec); if (str != NULL) { copy_params = alert_key2param(copy_params, PCMK__alert_key_timestamp, str); free(str); } - str = pcmk__assert_asprintf("%lld", (long long) epoch); + str = pcmk__assert_asprintf("%lld", (long long) now_tv.tv_sec); copy_params = alert_key2param(copy_params, PCMK__alert_key_timestamp_epoch, str); free(str); - str = pcmk__assert_asprintf("%06d", now->useconds); + str = pcmk__assert_asprintf("%06d", now_usec); copy_params = alert_key2param(copy_params, PCMK__alert_key_timestamp_usec, str); free(str); @@ -200,8 +203,6 @@ exec_alert_list(lrmd_t *lrmd, const GList *alert_list, } } - free(now); - if (any_failure) { return (any_success? -1 : -2); } From 0a7c3f5f3cbd5e578b5918177d88d8ca53476f39 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 8 Nov 2025 13:05:47 -0600 Subject: [PATCH 7/9] Refactor: libcrmcommon: Drop pcmk__time_hr_now() It has no remaining callers. Signed-off-by: Reid Wahl --- include/crm/common/iso8601_internal.h | 1 - lib/common/iso8601.c | 36 --------------------------- 2 files changed, 37 deletions(-) diff --git a/include/crm/common/iso8601_internal.h b/include/crm/common/iso8601_internal.h index 38e1c4cf358..2b1756fa496 100644 --- a/include/crm/common/iso8601_internal.h +++ b/include/crm/common/iso8601_internal.h @@ -30,7 +30,6 @@ typedef struct { int useconds; } pcmk__time_hr_t; -pcmk__time_hr_t *pcmk__time_hr_now(time_t *epoch); char *pcmk__time_format_hr(const char *format, const crm_time_t *dt, int usec); char *pcmk__epoch2str(const time_t *source, uint32_t flags); char *pcmk__timespec2str(const struct timespec *ts, uint32_t flags); diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index 5ea487688a4..8a0554d210d 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -1910,42 +1910,6 @@ crm_time_add_years(crm_time_t * a_time, int extra) * crm_time_t, pcmk__time_hr_t, and struct timespec (in lrmd_cmd_t). */ -/*! - * \internal - * \brief Return the current time as a high-resolution time - * - * \param[out] epoch If not NULL, this will be set to seconds since epoch - * - * \return Newly allocated high-resolution time set to the current time - * - * \note The return value is guaranteed not to be \c NULL. - * \note The caller is responsible for freeing the return value using \c free(). - */ -pcmk__time_hr_t * -pcmk__time_hr_now(time_t *epoch) -{ - struct timespec tv = { 0, }; - crm_time_t dt = { 0, }; - pcmk__time_hr_t *hr = pcmk__assert_alloc(1, sizeof(pcmk__time_hr_t)); - - qb_util_timespec_from_epoch_get(&tv); - - if (epoch != NULL) { - *epoch = tv.tv_sec; - } - - crm_time_set_timet(&dt, &(tv.tv_sec)); - - hr->years = dt.years; - hr->months = dt.months; - hr->days = dt.days; - hr->seconds = dt.seconds; - hr->offset = dt.offset; - hr->duration = dt.duration; - hr->useconds = tv.tv_nsec / QB_TIME_NS_IN_USEC; - return hr; -} - static void ha_get_tm_time(struct tm *target, const crm_time_t *source) { From 98894005ca2ebfd7d7ed6bf2a839440c16112cc1 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 8 Nov 2025 13:06:39 -0600 Subject: [PATCH 8/9] Refactor: libcrmcommon: Drop pcmk__time_hr_t Nothing uses it anymore. Signed-off-by: Reid Wahl --- include/crm/common/iso8601_internal.h | 10 ---------- lib/common/iso8601.c | 8 -------- 2 files changed, 18 deletions(-) diff --git a/include/crm/common/iso8601_internal.h b/include/crm/common/iso8601_internal.h index 2b1756fa496..54f89f008ea 100644 --- a/include/crm/common/iso8601_internal.h +++ b/include/crm/common/iso8601_internal.h @@ -20,16 +20,6 @@ extern "C" { #endif -typedef struct { - int years; - int months; // Only for durations - int days; - int seconds; - int offset; // In seconds - bool duration; - int useconds; -} pcmk__time_hr_t; - char *pcmk__time_format_hr(const char *format, const crm_time_t *dt, int usec); char *pcmk__epoch2str(const time_t *source, uint32_t flags); char *pcmk__timespec2str(const struct timespec *ts, uint32_t flags); diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index 8a0554d210d..5a75ab24f51 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -1902,14 +1902,6 @@ crm_time_add_years(crm_time_t * a_time, int extra) } } -/* The high-resolution variant of time object was added to meet an immediate - * need, and is kept internal API. - * - * @TODO The long-term goal is to come up with a clean, unified design for a - * time type (or types) that meets all the various needs, to replace - * crm_time_t, pcmk__time_hr_t, and struct timespec (in lrmd_cmd_t). - */ - static void ha_get_tm_time(struct tm *target, const crm_time_t *source) { From 54cc1cca18f5f117ff0b68b1af66c9db21d89dab Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Sat, 8 Nov 2025 16:25:40 -0600 Subject: [PATCH 9/9] Refactor: libcrmcommon: Drop ydays() macro It's basically redundant. Signed-off-by: Reid Wahl --- lib/common/iso8601.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/common/iso8601.c b/lib/common/iso8601.c index 5a75ab24f51..48964e599dd 100644 --- a/lib/common/iso8601.c +++ b/lib/common/iso8601.c @@ -151,12 +151,7 @@ crm_time_free(crm_time_t * dt) static int year_days(int year) { - int d = 365; - - if (crm_time_leapyear(year)) { - d++; - } - return d; + return crm_time_leapyear(year)? 366 : 365; } /* From http://myweb.ecu.edu/mccartyr/ISOwdALG.txt : @@ -1788,8 +1783,6 @@ crm_time_add_seconds(crm_time_t *a_time, int extra) crm_time_add_days(a_time, days); } -#define ydays(t) (crm_time_leapyear((t)->years)? 366 : 365) - /*! * \brief Add days to a date/time * @@ -1804,12 +1797,13 @@ crm_time_add_days(crm_time_t *a_time, int extra) crm_trace("Adding %d days to %.4d-%.3d", extra, a_time->years, a_time->days); if (extra > 0) { - while ((a_time->days + (long long) extra) > ydays(a_time)) { + while ((a_time->days + (long long) extra) > year_days(a_time->years)) { if ((a_time->years + 1LL) > INT_MAX) { - a_time->days = ydays(a_time); // Clip to latest we can handle + // Clip to latest we can handle + a_time->days = year_days(a_time->years); return; } - extra -= ydays(a_time); + extra -= year_days(a_time->years); a_time->years++; } } else if (extra < 0) { @@ -1821,7 +1815,7 @@ crm_time_add_days(crm_time_t *a_time, int extra) return; } a_time->years--; - extra += ydays(a_time); + extra += year_days(a_time->years); } } a_time->days += extra;