Refactor: libcrmcommon: New pcmk__time_parse_duration()

To replace crm_time_parse_duration().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

Author: nrwahl2

cts/cli/regression.dates.exp

@@ -64,7 +64,7 @@ iso8601: Invalid interval specified: P1Y/2019-02-29 00:00:00Z
 =#=#=#= End test: Invalid period - [P1Y/2019-02-29 00:00:00Z] - Invalid parameter (2) =#=#=#=
 * Passed: iso8601               - Invalid period - [P1Y/2019-02-29 00:00:00Z]
 =#=#=#= Begin test: Invalid period - [2019-01-01 00:00:00Z/P] =#=#=#=
-crm_time_parse_duration 	error: 'P' is not a valid ISO 8601 time duration because nothing follows 'P'
+pcmk__time_parse_duration 	error: 'P' is not a valid ISO 8601 time duration because nothing follows 'P'
 iso8601: Invalid interval specified: 2019-01-01 00:00:00Z/P
 =#=#=#= End test: Invalid period - [2019-01-01 00:00:00Z/P] - Invalid parameter (2) =#=#=#=
 * Passed: iso8601               - Invalid period - [2019-01-01 00:00:00Z/P]

cts/cts-cli.in

@@ -251,7 +251,6 @@ def sanitize_output(s):
         (r'(<cib.*) cib-last-written="[^"]*"', r'\1'),
         (r'crm_feature_set="[^"]*" ', r''),
         (r'@crm_feature_set=[0-9.]+, ', r''),
-        (r'\(crm_time_parse_duration@.*\.c:[0-9]+\)', r'crm_time_parse_duration'),
         (r'\(parse_hms@.*\.c:[0-9]+\)', r'parse_hms'),
         (re.escape(cts_cli_data), r'CTS_CLI_DATA'),
         (r' default="[^"]*"', r' default=""'),

include/crm/common/iso8601_internal.h

@@ -34,6 +34,8 @@ 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);
+
+crm_time_t *pcmk__time_parse_duration(const char *period_s);
 crm_time_t *pcmk__copy_timet(time_t source_sec);
 
 void pcmk__time_log_as(const char *file, const char *function, int line,

lib/common/iso8601.c

@@ -1282,18 +1282,19 @@ parse_duration_element(const char **element, const char *duration_s,
 }
 
 /*!
+ * \internal
  * \brief Parse a time duration from an ISO 8601 duration specification
  *
  * \param[in] period_s  ISO 8601 duration specification (optionally followed by
  *                      whitespace, after which the rest of the string will be
  *                      ignored)
  *
- * \return New time object on success, NULL (and set errno) otherwise
- * \note It is the caller's responsibility to return the result using
- *       crm_time_free().
+ * \return New time object on success, or \cNULL (and set \c errno) otherwise
+ * \note It is the caller's responsibility to free the result using
+ *       \c crm_time_free().
  */
 crm_time_t *
-crm_time_parse_duration(const char *period_s)
+pcmk__time_parse_duration(const char *period_s)
 {
     bool is_time = false;
     crm_time_t *diff = NULL;
@@ -1344,15 +1345,35 @@ crm_time_parse_duration(const char *period_s)
         goto invalid;
     }
 
-    diff->duration = TRUE;
+    diff->duration = true;
     return diff;
 
 invalid:
+    /* @COMPAT Setting errno is required only for backward compatibility with
+     * crm_time_parse_duration()
+     */
     crm_time_free(diff);
     errno = EINVAL;
     return NULL;
 }
 
+/*!
+ * \brief Parse a time duration from an ISO 8601 duration specification
+ *
+ * \param[in] period_s  ISO 8601 duration specification (optionally followed by
+ *                      whitespace, after which the rest of the string will be
+ *                      ignored)
+ *
+ * \return New time object on success, NULL (and set errno) otherwise
+ * \note It is the caller's responsibility to return the result using
+ *       crm_time_free().
+ */
+crm_time_t *
+crm_time_parse_duration(const char *period_s)
+{
+    return pcmk__time_parse_duration(period_s);
+}
+
 /*!
  * \internal
  * \brief Set one time object to another if the other is earlier
@@ -2329,7 +2350,7 @@ crm_time_parse_period(const char *period_str)
     period = pcmk__assert_alloc(1, sizeof(crm_time_period_t));
 
     if (period_str[0] == 'P') {
-        period->diff = crm_time_parse_duration(period_str);
+        period->diff = pcmk__time_parse_duration(period_str);
         if (period->diff == NULL) {
             goto invalid;
         }
@@ -2350,7 +2371,7 @@ crm_time_parse_period(const char *period_str)
                           original);
                 goto invalid;
             }
-            period->diff = crm_time_parse_duration(period_str);
+            period->diff = pcmk__time_parse_duration(period_str);
             if (period->diff == NULL) {
                 goto invalid;
             }

lib/common/strings.c

@@ -1,5 +1,5 @@
 /*
- * Copyright 2004-2025 the Pacemaker project contributors
+ * Copyright 2004-2026 the Pacemaker project contributors
  *
  * The version control history for this file may have further details.
  *
@@ -372,7 +372,7 @@ pcmk_parse_interval_spec(const char *input, guint *result_ms)
     }
 
     if (input[0] == 'P') {
-        crm_time_t *period_s = crm_time_parse_duration(input);
+        crm_time_t *period_s = pcmk__time_parse_duration(input);
 
         if (period_s != NULL) {
             msec = crm_time_get_seconds(period_s);

lib/common/tests/iso8601/Makefile.am

@@ -15,10 +15,10 @@ include $(top_srcdir)/mk/unittest.mk
 check_PROGRAMS = crm_time_add_days_test
 check_PROGRAMS += crm_time_add_seconds_test
 check_PROGRAMS += crm_time_add_years_test
-check_PROGRAMS += crm_time_parse_duration_test
 check_PROGRAMS += pcmk__add_time_from_xml_test
 check_PROGRAMS += pcmk__readable_interval_test
 check_PROGRAMS += pcmk__set_time_if_earlier_test
 check_PROGRAMS += pcmk__time_format_hr_test
+check_PROGRAMS += pcmk__time_parse_duration_test
 
 TESTS = $(check_PROGRAMS)

lib/common/tests/iso8601/crm_time_parse_duration_test.c

@@ -0,0 +1,110 @@
+/*
+ * Copyright 2024-2026 the Pacemaker project contributors
+ *
+ * The version control history for this file may have further details.
+ *
+ * This source code is licensed under the GNU General Public License version 2
+ * or later (GPLv2+) WITHOUT ANY WARRANTY.
+ */
+
+#include <crm_internal.h>
+
+#include <crm/common/unittest_internal.h>
+
+#include <crm/common/iso8601.h>
+#include "crmcommon_private.h"
+
+static void
+empty_arg(void **state)
+{
+    assert_null(pcmk__time_parse_duration(NULL));
+    assert_null(pcmk__time_parse_duration(""));
+}
+
+static void
+invalid_arg(void **state)
+{
+    // Valid except doesn't start with P
+    assert_null(pcmk__time_parse_duration("X3Y6M4DT12H30M5S"));
+
+    // Illegal character after P
+    assert_null(pcmk__time_parse_duration("P"));
+    assert_null(pcmk__time_parse_duration("P 3Y6M4DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("PX3Y6M4DT12H30M5S"));
+
+    // Missing or invalid units
+    assert_null(pcmk__time_parse_duration("P3Y6M4DT12H30M5"));
+    assert_null(pcmk__time_parse_duration("P3Y6M4DT12H30M5X"));
+    assert_null(pcmk__time_parse_duration("P3X6M4DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("PT"));
+    assert_null(pcmk__time_parse_duration("P/"));
+
+#if 0
+    // @TODO The current implementation treats these as valid
+
+    // Units out of order
+    assert_null(pcmk__time_parse_duration("P6M3Y4DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("P6M3DT12HY430M5S"));
+
+    // Same unit specified multiple times
+    assert_null(pcmk__time_parse_duration("P6Y4M3D1MT12H30M5S"));
+
+    // Weeks mixed with other units
+    assert_null(pcmk__time_parse_duration("P6Y4M3W3D1MT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3WT12H30M5S"));
+#endif
+}
+
+static void
+overflow(void **state)
+{
+    // Too large
+    assert_null(pcmk__time_parse_duration("P2147483648Y6M4DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y2147483648M4DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y6M2147483648DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y6M4DT2147483648H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y6M4DT12H2147483648M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y6M4DT12H30MP2147483648S"));
+
+    // Too small
+    assert_null(pcmk__time_parse_duration("P-2147483648Y6M4DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y-2147483648M4DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y6M-2147483648DT12H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y6M4DT-2147483648H30M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y6M4DT12H-2147483648M5S"));
+    assert_null(pcmk__time_parse_duration("P3Y6M4DT12H30MP-2147483648S"));
+}
+
+static void
+valid_arg(void **state)
+{
+    // @TODO Check result value
+    assert_non_null(pcmk__time_parse_duration("P3Y6M4DT12H30M5S"));
+    assert_non_null(pcmk__time_parse_duration("P3Y6M4DT12H30M-5S"));
+    assert_non_null(pcmk__time_parse_duration("P3Y6M4DT12H-30M5S"));
+    assert_non_null(pcmk__time_parse_duration("P3Y6M4DT-12H30M5S"));
+    assert_non_null(pcmk__time_parse_duration("P3Y6M-4DT12H30M5S"));
+    assert_non_null(pcmk__time_parse_duration("P3Y-6M4DT12H30M5S"));
+    assert_non_null(pcmk__time_parse_duration("P3Y6M4DT12H30M"));
+    assert_non_null(pcmk__time_parse_duration("P3Y6M4D"));
+    assert_non_null(pcmk__time_parse_duration("P1M"));  // 1 month
+    assert_non_null(pcmk__time_parse_duration("PT1M")); // 1 minute
+    assert_non_null(pcmk__time_parse_duration("P7W"));
+
+#if 0
+    // @TODO Current implementation can't handle these cases
+
+    // Fractional value for last unit
+    assert_non_null(pcmk__time_parse_duration("P3Y6M4DT12H30.5M"));
+    assert_non_null(pcmk__time_parse_duration("P3Y6M4DT12H30,5M"));
+
+    // P<YYYY>-<MM>-<DD>T<hh>:<mm>:<ss> format
+    assert_non_null(pcmk__time_parse_duration("P0003-02-01T11:10:09");
+#endif
+}
+
+PCMK__UNIT_TEST(NULL, NULL,
+                cmocka_unit_test(empty_arg),
+                cmocka_unit_test(invalid_arg),
+                cmocka_unit_test(overflow),
+                cmocka_unit_test(valid_arg));

lib/common/tests/iso8601/pcmk__time_parse_duration_test.c

@@ -31,7 +31,7 @@ parse_cli_lifetime(pcmk__output_t *out, const char *move_lifetime)
         return NULL;
     }
 
-    duration = crm_time_parse_duration(move_lifetime);
+    duration = pcmk__time_parse_duration(move_lifetime);
     if (duration == NULL) {
         out->err(out, "Invalid duration specified: %s\n"
                       "Please refer to https://en.wikipedia.org/wiki/ISO_8601#Durations "

tools/crm_resource_ban.c

@@ -295,7 +295,7 @@ parse_period(const char *period_str, crm_time_t **start, crm_time_t **end)
     tzset();
 
     if (period_str[0] == 'P') {
-        diff = crm_time_parse_duration(period_str);
+        diff = pcmk__time_parse_duration(period_str);
         if (diff == NULL) {
             goto invalid;
         }
@@ -315,7 +315,7 @@ parse_period(const char *period_str, crm_time_t **start, crm_time_t **end)
                           "has two durations", original);
                 goto invalid;
             }
-            diff = crm_time_parse_duration(period_str);
+            diff = pcmk__time_parse_duration(period_str);
             if (diff == NULL) {
                 goto invalid;
             }
@@ -467,7 +467,7 @@ main(int argc, char **argv)
     }
 
     if (options.duration_s) {
-        duration = crm_time_parse_duration(options.duration_s);
+        duration = pcmk__time_parse_duration(options.duration_s);
 
         if (duration == NULL) {
             exit_code = CRM_EX_INVALID_PARAM;