Replace uses of strtok() with g_strsplit() and friends

cts/cts-fencing.in

@@ -510,21 +510,28 @@ class FenceTests(Tests):
                      args='--output-as=xml -R true1 -a fence_dummy -o mode=pass -o "pcmk_host_list=node1 node2 node3" -o pcmk_delay_base=1')
         test.add_cmd("stonith_admin",
                      args='--output-as=xml -R false1 -a fence_dummy -o mode=fail -o "pcmk_host_list=node1 node2 node3" -o pcmk_delay_base=1')
-        # Resulting "random" delay will always be 1 since (rand() % (delay_max - delay_base)) is always 0 here
+
+        # Resulting "random" delay will be either 1 or 2 seconds
         test.add_cmd("stonith_admin",
                      args='--output-as=xml -R true2 -a fence_dummy -o mode=pass -o "pcmk_host_list=node1 node2 node3" -o pcmk_delay_base=1 -o pcmk_delay_max=2')
+
+        # Resulting delay will be 1 second (capped by pcmk_delay_max)
         test.add_cmd("stonith_admin",
-                     args='--output-as=xml -R true3 -a fence_dummy -o mode=pass -o "pcmk_host_list=node1 node2 node3"')
+                     args='--output-as=xml -R true3 -a fence_dummy -o mode=pass -o "pcmk_host_list=node1 node2 node3" -o pcmk_delay_base=2 -o pcmk_delay_max=1')
+
+        test.add_cmd("stonith_admin",
+                     args='--output-as=xml -R true4 -a fence_dummy -o mode=pass -o "pcmk_host_list=node1 node2 node3"')
 
         test.add_cmd("stonith_admin", args="--output-as=xml -r node3 -i 1 -v true1")
         test.add_cmd("stonith_admin", args="--output-as=xml -r node3 -i 1 -v false1")
         test.add_cmd("stonith_admin", args="--output-as=xml -r node3 -i 2 -v true2")
         test.add_cmd("stonith_admin", args="--output-as=xml -r node3 -i 2 -v true3")
+        test.add_cmd("stonith_admin", args="--output-as=xml -r node3 -i 2 -v true4")
 
         test.add_cmd("stonith_admin", args="--output-as=xml -F node3 --delay 1")
 
         # Total fencing timeout takes all fencing delays into account
-        test.add_log_pattern("Total timeout set to 582s")
+        test.add_log_pattern("Total timeout set to 727s")
 
         # Fencing timeout for the first device takes the requested fencing delay
         # and pcmk_delay_base into account
@@ -543,9 +550,15 @@ class FenceTests(Tests):
         # Fencing timeout takes pcmk_delay_max into account
         test.add_log_pattern(r"Requesting that .* perform 'off' action targeting node3 using true2 .*146s.*",
                              regex=True)
-        test.add_log_pattern("Delaying 'off' action targeting node3 using true2 for 1s | timeout=120s requested_delay=0s base=1s max=2s")
+        test.add_log_pattern(r"Delaying 'off' action targeting node3 using true2 for [12]s "
+                             "| timeout=120s requested_delay=0s base=1s max=2s",
+                             regex=True)
+        test.add_log_pattern(r"Requesting that .* perform 'off' action targeting node3 using true3 .*145s.*",
+                             regex=True)
+        test.add_log_pattern("Delaying 'off' action targeting node3 using true3 for 1s "
+                             "| timeout=120s requested_delay=0s base=1s max=1s")
 
-        test.add_log_pattern("Delaying 'off' action targeting node3 using true3",
+        test.add_log_pattern("Delaying 'off' action targeting node3 using true4",
                              negative=True)
 
     def build_unfence_tests(self):

daemons/fenced/fenced_commands.c

@@ -201,48 +201,110 @@ get_action_delay_max(const fenced_device_t *device, const char *action)
     return (int) delay_max;
 }
 
+static gchar *
+get_value_if_matching(const char *mapping, const char *target)
+{
+    gchar **nvpair = NULL;
+    gchar *value = NULL;
+
+    if (pcmk__str_empty(mapping)) {
+        goto done;
+    }
+
+    nvpair = g_strsplit(mapping, ":", 2);
+
+    if (pcmk__str_empty(nvpair[0]) || pcmk__str_empty(nvpair[1])) {
+        crm_err(PCMK_FENCING_DELAY_BASE ": Malformed mapping '%s'", mapping);
+        goto done;
+    }
+
+    if (!pcmk__str_eq(target, nvpair[0], pcmk__str_casei)) {
+        goto done;
+    }
+
+    // Take ownership so that we don't free nvpair[1] with nvpair
+    value = nvpair[1];
+    nvpair[1] = NULL;
+
+    crm_debug(PCMK_FENCING_DELAY_BASE " mapped to %s for %s", value, target);
+
+done:
+    g_strfreev(nvpair);
+    return value;
+}
+
+static gchar *
+get_value_for_target(const char *target, const char *values)
+{
+    gchar *value = NULL;
+    gchar **mappings = g_strsplit_set(values, "; \t", 0);
+
+    /* If there are no delimiters after stripping leading and trailing
+     * whitespace, then we want to parse the string as a single interval, rather
+     * than as a delimited list of mappings. Short-circuiting here when we split
+     * into fewer than two mappings avoids a "Malformed mapping" error message
+     * below.
+     */
+    if (g_strv_length(mappings) < 2) {
+        goto done;
+    }
+
+    for (gchar **mapping = mappings; *mapping != NULL; mapping++) {
+        value = get_value_if_matching(*mapping, target);
+        if (value != NULL) {
+            break;
+        }
+    }
+
+done:
+    g_strfreev(mappings);
+    return value;
+}
+
+/* @TODO Consolidate some of this with build_port_aliases(). But keep in mind
+ * that build_port_aliases()/pcmk__host_map supports either '=' or ':' as a
+ * mapping separator, while pcmk_delay_base supports only ':'.
+ */
 static int
 get_action_delay_base(const fenced_device_t *device, const char *action,
                       const char *target)
 {
-    char *hash_value = NULL;
+    const char *param = NULL;
+    gchar *stripped = NULL;
+    gchar *delay_base_s = NULL;
     guint delay_base = 0U;
 
     if (!pcmk__is_fencing_action(action)) {
         return 0;
     }
 
-    hash_value = g_hash_table_lookup(device->params, PCMK_FENCING_DELAY_BASE);
-
-    if (hash_value) {
-        char *value = pcmk__str_copy(hash_value);
-        char *valptr = value;
-
-        if (target != NULL) {
-            for (char *val = strtok(value, "; \t"); val != NULL; val = strtok(NULL, "; \t")) {
-                char *mapval = strchr(val, ':');
+    param = g_hash_table_lookup(device->params, PCMK_FENCING_DELAY_BASE);
+    if (param == NULL) {
+        return 0;
+    }
 
-                if (mapval == NULL || mapval[1] == 0) {
-                    crm_err("pcmk_delay_base: empty value in mapping", val);
-                    continue;
-                }
+    stripped = g_strstrip(g_strdup(param));
+    if (target != NULL) {
+        delay_base_s = get_value_for_target(target, stripped);
+    }
 
-                if (mapval != val && strncasecmp(target, val, (size_t)(mapval - val)) == 0) {
-                    value = mapval + 1;
-                    crm_debug("pcmk_delay_base mapped to %s for %s",
-                              value, target);
-                    break;
-                }
-            }
-        }
+    if (delay_base_s == NULL) {
+        /* Either target is NULL or we didn't find a mapping. Try to parse the
+         * stripped value itself. Take ownership so that we don't free stripped
+         * twice.
+         */
+        delay_base_s = stripped;
+        stripped = NULL;
+    }
 
-        if (strchr(value, ':') == 0) {
-            pcmk_parse_interval_spec(value, &delay_base);
-            delay_base /= 1000;
-        }
+    /* @TODO Should we accept only a simple time+units string, rather than an
+     * ISO 8601 interval?
+     */
+    pcmk_parse_interval_spec(delay_base_s, &delay_base);
+    delay_base /= 1000;
 
-        free(valptr);
-    }
+    g_free(stripped);
+    g_free(delay_base_s);
 
     return (int) delay_base;
 }
@@ -839,82 +901,43 @@ fenced_free_device_table(void)
 }
 
 static GHashTable *
-build_port_aliases(const char *hostmap, GList ** targets)
+build_port_aliases(const char *hostmap, GList **targets)
 {
-    char *name = NULL;
-    int last = 0, lpc = 0, max = 0, added = 0;
     GHashTable *aliases = pcmk__strikey_table(free, free);
+    gchar *stripped = NULL;
+    gchar **mappings = NULL;
 
-    if (hostmap == NULL) {
-        return aliases;
+    if (pcmk__str_empty(hostmap)) {
+        goto done;
     }
 
-    max = strlen(hostmap);
-    for (; lpc <= max; lpc++) {
-        switch (hostmap[lpc]) {
-                /* Skip escaped chars */
-            case '\\':
-                lpc++;
-                break;
+    stripped = g_strstrip(g_strdup(hostmap));
+    mappings = g_strsplit_set(stripped, "; \t", 0);
 
-                /* Assignment chars */
-            case '=':
-            case ':':
-                if (lpc > last) {
-                    free(name);
-                    name = pcmk__assert_alloc(1, 1 + lpc - last);
-                    memcpy(name, hostmap + last, lpc - last);
-                }
-                last = lpc + 1;
-                break;
+    for (gchar **mapping = mappings; *mapping != NULL; mapping++) {
+        gchar **nvpair = NULL;
 
-                /* Delimeter chars */
-                /* case ',': Potentially used to specify multiple ports */
-            case 0:
-            case ';':
-            case ' ':
-            case '\t':
-                if (name) {
-                    char *value = NULL;
-                    int k = 0;
-
-                    value = pcmk__assert_alloc(1, 1 + lpc - last);
-                    memcpy(value, hostmap + last, lpc - last);
-
-                    for (int i = 0; value[i] != '\0'; i++) {
-                        if (value[i] != '\\') {
-                            value[k++] = value[i];
-                        }
-                    }
-                    value[k] = '\0';
-
-                    crm_debug("Adding alias '%s'='%s'", name, value);
-                    g_hash_table_replace(aliases, name, value);
-                    if (targets) {
-                        *targets = g_list_append(*targets, pcmk__str_copy(value));
-                    }
-                    value = NULL;
-                    name = NULL;
-                    added++;
+        if (pcmk__str_empty(*mapping)) {
+            continue;
+        }
 
-                } else if (lpc > last) {
-                    crm_debug("Parse error at offset %d near '%s'", lpc - last, hostmap + last);
-                }
+        // @COMPAT Drop support for '=' as delimiter
+        nvpair = g_strsplit_set(*mapping, ":=", 2);
 
-                last = lpc + 1;
-                break;
-        }
+        if (pcmk__str_empty(nvpair[0]) || pcmk__str_empty(nvpair[1])) {
+            crm_err(PCMK_FENCING_HOST_MAP ": Malformed mapping '%s'", *mapping);
 
-        if (hostmap[lpc] == 0) {
-            break;
+        } else {
+            crm_debug("Adding alias '%s'='%s'", nvpair[0], nvpair[1]);
+            pcmk__insert_dup(aliases, nvpair[0], nvpair[1]);
+            *targets = g_list_append(*targets, pcmk__str_copy(nvpair[1]));
         }
+        g_strfreev(nvpair);
     }
 
-    if (added == 0) {
-        crm_info("No host mappings detected in '%s'", hostmap);
-    }
-
-    free(name);
+done:
+    g_free(stripped);
+    g_strfreev(mappings);
     return aliases;
 }
 

include/crm/services.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2024 the Pacemaker project contributors
+ * Copyright 2010-2025 the Pacemaker project contributors
  *
  * The version control history for this file may have further details.
  *
@@ -159,20 +159,6 @@ typedef struct svc_action_s {
     svc_action_private_t *opaque;
 } svc_action_t;
 
-/*!
- * \brief Get a list of files or directories in a given path
- *
- * \param[in] root       Full path to a directory to read
- * \param[in] files      Return list of files if TRUE or directories if FALSE
- * \param[in] executable If TRUE and files is TRUE, only return executable files
- *
- * \return List of what was found as char * items.
- * \note The caller is responsibile for freeing the result with
- *       g_list_free_full(list, free).
- */
-GList *get_directory_list(const char *root, gboolean files,
-                          gboolean executable);
-
 /*!
  * \brief Get a list of providers
  *

include/crm/services_compat.h

@@ -10,6 +10,8 @@
 #ifndef PCMK__CRM_SERVICES_COMPAT__H
 #define PCMK__CRM_SERVICES_COMPAT__H
 
+#include <glib.h>               // GList, gboolean
+
 #include <crm/common/results.h> // enum ocf_exitcode, PCMK_OCF_OK, etc.
 
 #ifdef __cplusplus
@@ -50,6 +52,10 @@ services_ocf_exitcode_str(enum ocf_exitcode code)
     }
 }
 
+//! \deprecated Do not use
+GList *get_directory_list(const char *root, gboolean files,
+                          gboolean executable);
+
 #  ifdef __cplusplus
 }
 #  endif

lib/cluster/cpg.c

@@ -466,7 +466,7 @@ pcmk__cpg_message_data(cpg_handle_t handle, uint32_t sender_id, uint32_t pid,
     if (msg->is_compressed && (msg->size > 0)) {
         int rc = BZ_OK;
         unsigned int new_size = msg->size + 1;
-        char *uncompressed = pcmk__assert_alloc(1, new_size);
+        char *uncompressed = pcmk__assert_alloc(new_size, sizeof(char));
 
         rc = BZ2_bzBuffToBuffDecompress(uncompressed, &new_size, msg->data,
                                         msg->compressed_size, 1, 0);

lib/common/actions.c

@@ -403,7 +403,7 @@ decode_transition_magic(const char *magic, char **uuid, int *transition_id, int
     res = sscanf(magic, "%d:%d;%ms", &local_op_status, &local_op_rc, &key);
 #else
     // magic must have >=4 other characters
-    key = pcmk__assert_alloc(1, strlen(magic) - 3);
+    key = pcmk__assert_alloc(strlen(magic) - 3, sizeof(char));
     res = sscanf(magic, "%d:%d;%s", &local_op_status, &local_op_rc, key);
 #endif
     if (res == EOF) {

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.
  *
@@ -27,7 +27,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
     if (size < 10) {
         return -1; // Do not add input to testing corpus
     }
-    ns = pcmk__assert_alloc(1, size + 1);
+    ns = pcmk__assert_alloc(size + 1, sizeof(char));
     memcpy(ns, data, size);
 
     period = crm_time_parse_period(ns);

lib/common/fuzzers/scores_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.
  *
@@ -21,7 +21,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
     guint result = 0U;
 
     if (size > 0) {
-        ns = pcmk__assert_alloc(1, size + 1);
+        ns = pcmk__assert_alloc(size + 1, sizeof(char));
         memcpy(ns, data, size);
         ns[size] = '\0';
     }