Merge pull request #3387 from kgaillot/best-practices

Move some rule support functions to libcrmcommon

include/crm/common/rules_internal.h

@@ -16,7 +16,43 @@
 #include <crm/common/rules.h>           // enum expression_type
 #include <crm/common/iso8601.h>         // crm_time_t
 
+// How node attribute values may be compared in rules
+enum pcmk__comparison {
+    pcmk__comparison_unknown,
+    pcmk__comparison_defined,
+    pcmk__comparison_undefined,
+    pcmk__comparison_eq,
+    pcmk__comparison_ne,
+    pcmk__comparison_lt,
+    pcmk__comparison_lte,
+    pcmk__comparison_gt,
+    pcmk__comparison_gte,
+};
+
+// How node attribute values may be parsed in rules
+enum pcmk__type {
+    pcmk__type_unknown,
+    pcmk__type_string,
+    pcmk__type_integer,
+    pcmk__type_number,
+    pcmk__type_version,
+};
+
+// Where to obtain reference value for a node attribute comparison
+enum pcmk__reference_source {
+    pcmk__source_unknown,
+    pcmk__source_literal,
+    pcmk__source_instance_attrs,
+    pcmk__source_meta_attrs,
+};
+
 enum expression_type pcmk__expression_type(const xmlNode *expr);
+enum pcmk__comparison pcmk__parse_comparison(const char *op);
+enum pcmk__type pcmk__parse_type(const char *type, enum pcmk__comparison op,
+                                 const char *value1, const char *value2);
+enum pcmk__reference_source pcmk__parse_source(const char *source);
+int pcmk__cmp_by_type(const char *value1, const char *value2,
+                      enum pcmk__type type);
 char *pcmk__replace_submatches(const char *string, const char *match,
                                const regmatch_t submatches[], int nmatches);
 

lib/common/rules.c

@@ -705,3 +705,186 @@ pcmk__replace_submatches(const char *string, const char *match,
                               NULL);
     return result;
 }
+
+/*!
+ * \internal
+ * \brief Parse a comparison type from a string
+ *
+ * \param[in] op  String with comparison type (valid values are
+ *                \c PCMK_VALUE_DEFINED, \c PCMK_VALUE_NOT_DEFINED,
+ *                \c PCMK_VALUE_EQ, \c PCMK_VALUE_NE,
+ *                \c PCMK_VALUE_LT, \c PCMK_VALUE_LTE,
+ *                \c PCMK_VALUE_GT, or \c PCMK_VALUE_GTE)
+ *
+ * \return Comparison type corresponding to \p op
+ */
+enum pcmk__comparison
+pcmk__parse_comparison(const char *op)
+{
+    if (pcmk__str_eq(op, PCMK_VALUE_DEFINED, pcmk__str_casei)) {
+        return pcmk__comparison_defined;
+
+    } else if (pcmk__str_eq(op, PCMK_VALUE_NOT_DEFINED, pcmk__str_casei)) {
+        return pcmk__comparison_undefined;
+
+    } else if (pcmk__str_eq(op, PCMK_VALUE_EQ, pcmk__str_casei)) {
+        return pcmk__comparison_eq;
+
+    } else if (pcmk__str_eq(op, PCMK_VALUE_NE, pcmk__str_casei)) {
+        return pcmk__comparison_ne;
+
+    } else if (pcmk__str_eq(op, PCMK_VALUE_LT, pcmk__str_casei)) {
+        return pcmk__comparison_lt;
+
+    } else if (pcmk__str_eq(op, PCMK_VALUE_LTE, pcmk__str_casei)) {
+        return pcmk__comparison_lte;
+
+    } else if (pcmk__str_eq(op, PCMK_VALUE_GT, pcmk__str_casei)) {
+        return pcmk__comparison_gt;
+
+    } else if (pcmk__str_eq(op, PCMK_VALUE_GTE, pcmk__str_casei)) {
+        return pcmk__comparison_gte;
+    }
+
+    return pcmk__comparison_unknown;
+}
+
+/*!
+ * \internal
+ * \brief Parse a value type from a string
+ *
+ * \param[in] type    String with value type (valid values are NULL,
+ *                    \c PCMK_VALUE_STRING, \c PCMK_VALUE_INTEGER,
+ *                    \c PCMK_VALUE_NUMBER, and \c PCMK_VALUE_VERSION)
+ * \param[in] op      Operation type (used only to select default)
+ * \param[in] value1  First value being compared (used only to select default)
+ * \param[in] value2  Second value being compared (used only to select default)
+ */
+enum pcmk__type
+pcmk__parse_type(const char *type, enum pcmk__comparison op,
+                 const char *value1, const char *value2)
+{
+    if (type == NULL) {
+        switch (op) {
+            case pcmk__comparison_lt:
+            case pcmk__comparison_lte:
+            case pcmk__comparison_gt:
+            case pcmk__comparison_gte:
+                if (((value1 != NULL) && (strchr(value1, '.') != NULL))
+                    || ((value2 != NULL) && (strchr(value2, '.') != NULL))) {
+                    return pcmk__type_number;
+                }
+                return pcmk__type_integer;
+
+            default:
+                return pcmk__type_string;
+        }
+    }
+
+    if (pcmk__str_eq(type, PCMK_VALUE_STRING, pcmk__str_casei)) {
+        return pcmk__type_string;
+
+    } else if (pcmk__str_eq(type, PCMK_VALUE_INTEGER, pcmk__str_casei)) {
+        return pcmk__type_integer;
+
+    } else if (pcmk__str_eq(type, PCMK_VALUE_NUMBER, pcmk__str_casei)) {
+        return pcmk__type_number;
+
+    } else if (pcmk__str_eq(type, PCMK_VALUE_VERSION, pcmk__str_casei)) {
+        return pcmk__type_version;
+    }
+
+    return pcmk__type_unknown;
+}
+
+/*!
+ * \internal
+ * \brief Compare two strings according to a given type
+ *
+ * \param[in] value1  String with first value to compare
+ * \param[in] value2  String with second value to compare
+ * \param[in] type    How to interpret the values
+ *
+ * \return Standard comparison result (a negative integer if \p value1 is
+ *         lesser, 0 if the values are equal, and a positive integer if
+ *         \p value1 is greater)
+ */
+int
+pcmk__cmp_by_type(const char *value1, const char *value2, enum pcmk__type type)
+{
+    //  NULL compares as less than non-NULL
+    if (value2 == NULL) {
+        return (value1 == NULL)? 0 : 1;
+    }
+    if (value1 == NULL) {
+        return -1;
+    }
+
+    switch (type) {
+        case pcmk__type_string:
+            return strcasecmp(value1, value2);
+
+        case pcmk__type_integer:
+            {
+                long long integer1;
+                long long integer2;
+
+                if ((pcmk__scan_ll(value1, &integer1, 0LL) != pcmk_rc_ok)
+                    || (pcmk__scan_ll(value2, &integer2, 0LL) != pcmk_rc_ok)) {
+                    crm_warn("Comparing '%s' and '%s' as strings because "
+                             "invalid as integers", value1, value2);
+                    return strcasecmp(value1, value2);
+                }
+                return (integer1 < integer2)? -1 : (integer1 > integer2)? 1 : 0;
+            }
+            break;
+
+        case pcmk__type_number:
+            {
+                double num1;
+                double num2;
+
+                if ((pcmk__scan_double(value1, &num1, NULL, NULL) != pcmk_rc_ok)
+                    || (pcmk__scan_double(value2, &num2, NULL,
+                                          NULL) != pcmk_rc_ok)) {
+                    crm_warn("Comparing '%s' and '%s' as strings because invalid as "
+                             "numbers", value1, value2);
+                    return strcasecmp(value1, value2);
+                }
+                return (num1 < num2)? -1 : (num1 > num2)? 1 : 0;
+            }
+            break;
+
+        case pcmk__type_version:
+            return compare_version(value1, value2);
+
+        default: // Invalid type
+            return 0;
+    }
+}
+
+/*!
+ * \internal
+ * \brief Parse a reference value source from a string
+ *
+ * \param[in] source  String indicating reference value source
+ *
+ * \return Reference value source corresponding to \p source
+ */
+enum pcmk__reference_source
+pcmk__parse_source(const char *source)
+{
+    if (pcmk__str_eq(source, PCMK_VALUE_LITERAL,
+                     pcmk__str_casei|pcmk__str_null_matches)) {
+        return pcmk__source_literal;
+
+    } else if (pcmk__str_eq(source, PCMK_VALUE_PARAM, pcmk__str_casei)) {
+        return pcmk__source_instance_attrs;
+
+    } else if (pcmk__str_eq(source, PCMK_VALUE_META, pcmk__str_casei)) {
+        return pcmk__source_meta_attrs;
+
+    } else {
+        return pcmk__source_unknown;
+    }
+}

lib/common/strings.c

@@ -1058,44 +1058,6 @@ pcmk__str_any_of(const char *s, ...)
     return rc;
 }
 
-/*!
- * \internal
- * \brief Check whether a character is in any of a list of strings
- *
- * \param[in]   ch      Character (ASCII) to search for
- * \param[in]   ...     Strings to search. Final argument must be
- *                      \c NULL.
- *
- * \return  \c true if any of \p ... contain \p ch, \c false otherwise
- * \note    \p ... must contain at least one argument (\c NULL).
- */
-bool
-pcmk__char_in_any_str(int ch, ...)
-{
-    bool rc = false;
-    va_list ap;
-
-    /*
-     * Passing a char to va_start() can generate compiler warnings,
-     * so ch is declared as an int.
-     */
-    va_start(ap, ch);
-
-    while (1) {
-        const char *ele = va_arg(ap, const char *);
-
-        if (ele == NULL) {
-            break;
-        } else if (strchr(ele, ch) != NULL) {
-            rc = true;
-            break;
-        }
-    }
-
-    va_end(ap);
-    return rc;
-}
-
 /*!
  * \internal
  * \brief Sort strings, with numeric portions sorted numerically

lib/common/tests/rules/Makefile.am

@@ -11,8 +11,12 @@ include $(top_srcdir)/mk/tap.mk
 include $(top_srcdir)/mk/unittest.mk
 
 # Add "_test" to the end of all test program names to simplify .gitignore.
-check_PROGRAMS = pcmk__evaluate_date_expression_test	\
+check_PROGRAMS = pcmk__cmp_by_type_test			\
+		 pcmk__evaluate_date_expression_test	\
 		 pcmk__evaluate_date_spec_test		\
+		 pcmk__parse_comparison_test		\
+		 pcmk__parse_source_test		\
+		 pcmk__parse_type_test			\
 		 pcmk__replace_submatches_test		\
 		 pcmk__unpack_duration_test
 

lib/common/tests/rules/pcmk__cmp_by_type_test.c

@@ -0,0 +1,101 @@
+/*
+ * Copyright 2024 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 <limits.h>                         // INT_MIN, INT_MAX
+
+#include <crm/common/util.h>                // crm_strdup_printf()
+#include <crm/common/rules_internal.h>
+#include <crm/common/unittest_internal.h>
+
+static void
+null_compares_lesser(void **state)
+{
+    assert_int_equal(pcmk__cmp_by_type(NULL, NULL, pcmk__type_string), 0);
+    assert_true(pcmk__cmp_by_type("0", NULL, pcmk__type_integer) > 0);
+    assert_true(pcmk__cmp_by_type(NULL, "0", pcmk__type_number) < 0);
+}
+
+static void
+invalid_compares_equal(void **state)
+{
+    assert_int_equal(pcmk__cmp_by_type("0", "1", pcmk__type_unknown), 0);
+    assert_int_equal(pcmk__cmp_by_type("hi", "bye", pcmk__type_unknown), 0);
+    assert_int_equal(pcmk__cmp_by_type("-1.0", "2.0", pcmk__type_unknown), 0);
+}
+
+static void
+compare_string_type(void **state)
+{
+    assert_int_equal(pcmk__cmp_by_type("bye", "bye", pcmk__type_string), 0);
+    assert_int_equal(pcmk__cmp_by_type("bye", "BYE", pcmk__type_string), 0);
+    assert_true(pcmk__cmp_by_type("bye", "hello", pcmk__type_string) < 0);
+    assert_true(pcmk__cmp_by_type("bye", "HELLO", pcmk__type_string) < 0);
+    assert_true(pcmk__cmp_by_type("bye", "boo", pcmk__type_string) > 0);
+    assert_true(pcmk__cmp_by_type("bye", "Boo", pcmk__type_string) > 0);
+}
+
+static void
+compare_integer_type(void **state)
+{
+    char *int_min = crm_strdup_printf("%d", INT_MIN);
+    char *int_max = crm_strdup_printf("%d", INT_MAX);
+
+    assert_int_equal(pcmk__cmp_by_type("0", "0", pcmk__type_integer), 0);
+    assert_true(pcmk__cmp_by_type("0", "1", pcmk__type_integer) < 0);
+    assert_true(pcmk__cmp_by_type("1", "0", pcmk__type_integer) > 0);
+    assert_true(pcmk__cmp_by_type("3999", "399", pcmk__type_integer) > 0);
+    assert_true(pcmk__cmp_by_type(int_min, int_max, pcmk__type_integer) < 0);
+    assert_true(pcmk__cmp_by_type(int_max, int_min, pcmk__type_integer) > 0);
+    free(int_min);
+    free(int_max);
+
+    // Non-integers compare as strings
+    assert_int_equal(pcmk__cmp_by_type("0", "x", pcmk__type_integer),
+                     pcmk__cmp_by_type("0", "x", pcmk__type_string));
+    assert_int_equal(pcmk__cmp_by_type("x", "0", pcmk__type_integer),
+                     pcmk__cmp_by_type("x", "0", pcmk__type_string));
+    assert_int_equal(pcmk__cmp_by_type("x", "X", pcmk__type_integer),
+                     pcmk__cmp_by_type("x", "X", pcmk__type_string));
+}
+
+static void
+compare_number_type(void **state)
+{
+    assert_int_equal(pcmk__cmp_by_type("0", "0.0", pcmk__type_number), 0);
+    assert_true(pcmk__cmp_by_type("0.345", "0.5", pcmk__type_number) < 0);
+    assert_true(pcmk__cmp_by_type("5", "3.1", pcmk__type_number) > 0);
+    assert_true(pcmk__cmp_by_type("3999", "399", pcmk__type_number) > 0);
+
+    // Non-numbers compare as strings
+    assert_int_equal(pcmk__cmp_by_type("0.0", "x", pcmk__type_number),
+                     pcmk__cmp_by_type("0.0", "x", pcmk__type_string));
+    assert_int_equal(pcmk__cmp_by_type("x", "0.0", pcmk__type_number),
+                     pcmk__cmp_by_type("x", "0.0", pcmk__type_string));
+    assert_int_equal(pcmk__cmp_by_type("x", "X", pcmk__type_number),
+                     pcmk__cmp_by_type("x", "X", pcmk__type_string));
+}
+
+static void
+compare_version_type(void **state)
+{
+    assert_int_equal(pcmk__cmp_by_type("1.0", "1.0", pcmk__type_version), 0);
+    assert_true(pcmk__cmp_by_type("1.0.0", "1.0.1", pcmk__type_version) < 0);
+    assert_true(pcmk__cmp_by_type("5.0", "3.1.15", pcmk__type_version) > 0);
+    assert_true(pcmk__cmp_by_type("3999", "399", pcmk__type_version) > 0);
+}
+
+PCMK__UNIT_TEST(NULL, NULL,
+                cmocka_unit_test(null_compares_lesser),
+                cmocka_unit_test(invalid_compares_equal),
+                cmocka_unit_test(compare_string_type),
+                cmocka_unit_test(compare_integer_type),
+                cmocka_unit_test(compare_number_type),
+                cmocka_unit_test(compare_version_type))