Feature: fencer: Improve validation of pcmk_host_map

Previously, if we hit a parsing error in pcmk_host_map, we logged a
debug message and continued trying to parse the rest of the value. It's
hard to be confident about the resulting behavior.

Now, we stop parsing if we encounter an error.

Also as part of this commit, we use g_strsplit_set() twice. At the first
level, we use it to split the list of node-name-to-port mappings in
pcmk_host_map, using ';', ' ', and '\t' as delimiters. At the second
level, we use it to split each mapping, using '=' and ':' as delimiters.
Support for '=' is likely to be removed in a future release.

I greatly struggled to reason about the existing code based on
"last = i + 1". It felt as if there were lots of potential edge cases.
The behavior may not be identical as of this commit, but it should be
more robust and predictable, and the code should be easier to maintain.

Signed-off-by: Reid Wahl <[email protected]>

Author: nrwahl2

daemons/fenced/fenced_commands.c

@@ -901,63 +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;
-    size_t len = 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;
     }
 
-    len = strlen(hostmap);
-    for (int i = 0; i <= len; i++) {
-        switch (hostmap[i]) {
-                /* Assignment chars */
-            case '=':
-            case ':':
-                if (i > last) {
-                    free(name);
-                    name = pcmk__assert_alloc(1 + i - last, sizeof(char));
-                    memcpy(name, hostmap + last, i - last);
-                }
-                last = i + 1;
-                break;
+    stripped = g_strstrip(g_strdup(hostmap));
+    mappings = g_strsplit_set(stripped, "; \t", 0);
 
-                /* Delimeter chars */
-                /* case ',': Potentially used to specify multiple ports */
-            case 0:
-            case ';':
-            case ' ':
-            case '\t':
-                if (name) {
-                    char *value = pcmk__assert_alloc(1 + i - last,
-                                                     sizeof(char));
-
-                    memcpy(value, hostmap + last, i - last);
-
-                    crm_debug("Adding alias '%s'='%s'", name, value);
-                    g_hash_table_replace(aliases, name, value);
-                    *targets = g_list_append(*targets, pcmk__str_copy(value));
-                    name = NULL;
-
-                } else if (i > last) {
-                    crm_debug("Parse error at offset %d near '%s'", i - last,
-                              hostmap + last);
-                }
+    for (gchar **mapping = mappings; *mapping != NULL; mapping++) {
+        gchar **nvpair = NULL;
 
-                last = i + 1;
-                break;
+        if (pcmk__str_empty(*mapping)) {
+            continue;
         }
 
-        if (hostmap[i] == 0) {
-            break;
+        // @COMPAT Drop support for '=' as delimiter
+        nvpair = g_strsplit_set(*mapping, ":=", 2);
+
+        if (pcmk__str_empty(nvpair[0]) || pcmk__str_empty(nvpair[1])) {
+            crm_err(PCMK_FENCING_HOST_MAP ": Malformed mapping '%s'", *mapping);
+
+        } 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);
     }
 
-    free(name);
+done:
+    g_free(stripped);
+    g_strfreev(mappings);
     return aliases;
 }