MINOR: cfgcond: start to split the condition parser to introduce terms

The purpose is to build a descendent parser that will split conditions
into expressions made of terms. There are two phases, a parsing phase
and an evaluation phase. Strictly speaking it's not required to cut
that in two right now, but it's likely that in the future we won't want
certain predicates to be evaluated during the parsing (e.g. file system
checks or execution of some external commands).

The cfg_eval_condition() function is now much simpler, it just tries to
parse a single term, and if OK evaluates it, then returns the result.
Errors are unchanged and may still be reported during parsing or
evaluation.

It's worth noting that some invalid expressions such as streq(a,b)zzz
continue to parse correctly for now (what remains after the parenthesis
is simply ignored as not necessary).

diff --git a/include/haproxy/cfgcond-t.h b/include/haproxy/cfgcond-t.h
index 04c8df16ef7b85d2dba53a01f166904fa889a3c8..ee9fbbede8cb89652d94e43c361732566cb380f3 100644 (file)
--- a/include/haproxy/cfgcond-t.h
+++ b/include/haproxy/cfgcond-t.h
@@ -52,6 +52,14 @@ enum cond_predicate {
        CFG_PRED_VERSION_BEFORE,  // "version_before"
 };
 
+/* types for condition terms */
+enum cfg_cond_term_type {
+       CCTT_NONE = 0,
+       CCTT_FALSE,
+       CCTT_TRUE,
+       CCTT_PRED,
+};
+
 /* keyword for a condition predicate */
 struct cond_pred_kw {
        const char *word;         // NULL marks the end of the list
@@ -59,4 +67,13 @@ struct cond_pred_kw {
        uint64_t arg_mask;        // mask of supported arguments (strings only)
 };
 
+/* condition term */
+struct cfg_cond_term {
+       enum cfg_cond_term_type type; // CCTT_*
+       struct arg *args;             // arguments for predicates
+       union {
+               const struct cond_pred_kw *pred; // predicate (function)
+       };
+};
+
 #endif /* _HAPROXY_CFGCOND_T_H */

diff --git a/include/haproxy/cfgcond.h b/include/haproxy/cfgcond.h
index bf8f58df6331c777af797f5666f566abe8884d21..0891176c92d139d808dc9b1e367fa12b0dbeac46 100644 (file)
--- a/include/haproxy/cfgcond.h
+++ b/include/haproxy/cfgcond.h
@@ -26,6 +26,8 @@
 #include <haproxy/cfgcond-t.h>
 
 const struct cond_pred_kw *cfg_lookup_cond_pred(const char *str);
+int cfg_parse_cond_term(const char **text, struct cfg_cond_term *term, char **err, const char **errptr);
+int cfg_eval_cond_term(const struct cfg_cond_term *term, char **err);
 int cfg_eval_condition(char **args, char **err, const char **errptr);
 
 #endif

diff --git a/src/cfgcond.c b/src/cfgcond.c
index d3c087b6648daae4c17c4929b05925c3796bfce1..df8e4d064287f244d5ebb334840193d15d66a174 100644 (file)
--- a/src/cfgcond.c
+++ b/src/cfgcond.c
@@ -45,103 +45,167 @@ const struct cond_pred_kw *cfg_lookup_cond_pred(const char *str)
        return NULL;
 }
 
-/* evaluate a condition on a .if/.elif line. The condition is already tokenized
- * in <err>. Returns -1 on error (in which case err is filled with a message,
- * and only in this case), 0 if the condition is false, 1 if it's true. If
- * <errptr> is not NULL, it's set to the first invalid character on error.
+/* Parse an indirect input text as a possible config condition term.
+ * Returns <0 on parsing error, 0 if the parser is desynchronized, or >0 on
+ * success. <term> is filled with the parsed info, and <text> is updated on
+ * success to point to the first unparsed character, or is left untouched
+ * on failure. On success, the caller must free term->args using free_args()
+ * and free the array itself. An error will be set in <err> on error, and only
+ * in this case. In this case the first bad character will be reported in
+ * <errptr>.
  */
-int cfg_eval_condition(char **args, char **err, const char **errptr)
+int cfg_parse_cond_term(const char **text, struct cfg_cond_term *term, char **err, const char **errptr)
 {
-       const struct cond_pred_kw *cond_pred = NULL;
+       const char *in = *text;
        const char *end_ptr;
-       struct arg *argp = NULL;
        int err_arg;
        int nbargs;
-       int ret = -1;
        char *end;
        long val;
 
-       if (!*args[0]) /* note: empty = false */
+       term->type = CCTT_NONE;
+       term->args = NULL;
+
+       while (*in == ' ' || *in == '\t')
+               in++;
+
+       if (!*in) /* empty term does not parse */
                return 0;
 
-       val = strtol(args[0], &end, 0);
-       if (end && *end == '\0')
-               return val != 0;
+       val = strtol(in, &end, 0);
+       if (end != in) {
+               term->type = val ? CCTT_TRUE : CCTT_FALSE;
+               *text = end;
+               return 1;
+       }
 
        /* below we'll likely all make_arg_list() so we must return only via
         * the <done> label which frees the arg list.
         */
-       cond_pred = cfg_lookup_cond_pred(args[0]);
-       if (cond_pred) {
-               nbargs = make_arg_list(args[0] + strlen(cond_pred->word), -1,
-                                      cond_pred->arg_mask, &argp, err,
+       term->pred = cfg_lookup_cond_pred(in);
+       if (term->pred) {
+               term->type = CCTT_PRED;
+               nbargs = make_arg_list(in + strlen(term->pred->word), -1,
+                                      term->pred->arg_mask, &term->args, err,
                                       &end_ptr, &err_arg, NULL);
-
                if (nbargs < 0) {
-                       memprintf(err, "%s in argument %d of predicate '%s' used in conditional expression", *err, err_arg, cond_pred->word);
+                       free_args(term->args);
+                       ha_free(&term->args);
+                       memprintf(err, "%s in argument %d of predicate '%s' used in conditional expression", *err, err_arg, term->pred->word);
                        if (errptr)
                                *errptr = end_ptr;
-                       goto done;
+                       return -1;
                }
+               *text = end_ptr;
+               return 1;
+       }
 
-               /* here we know we have a valid predicate with <nbargs> valid
-                * arguments, placed in <argp> (which we'll need to free).
+       memprintf(err, "unparsable conditional expression '%s'", *text);
+       if (errptr)
+               *errptr = *text;
+       return -1;
+}
+
+/* evaluate a condition term on a .if/.elif line. The condition was already
+ * parsed in <term>. Returns -1 on error (in which case err is filled with a
+ * message, and only in this case), 0 if the condition is false, 1 if it's
+ * true.
+ */
+int cfg_eval_cond_term(const struct cfg_cond_term *term, char **err)
+{
+       int ret = -1;
+
+       if (term->type == CCTT_FALSE)
+               ret = 0;
+       else if (term->type == CCTT_TRUE)
+               ret = 1;
+       else if (term->type == CCTT_PRED) {
+               /* here we know we have a valid predicate with valid arguments
+                * placed in term->args (which the caller will free).
                 */
-               switch (cond_pred->prd) {
+               switch (term->pred->prd) {
                case CFG_PRED_DEFINED:  // checks if arg exists as an environment variable
-                       ret = getenv(argp[0].data.str.area) != NULL;
-                       goto done;
+                       ret = getenv(term->args[0].data.str.area) != NULL;
+                       break;
 
                case CFG_PRED_FEATURE: { // checks if the arg matches an enabled feature
                        const char *p;
 
-                       for (p = build_features; (p = strstr(p, argp[0].data.str.area)); p++) {
-                               if ((p[argp[0].data.str.data] == ' ' || p[argp[0].data.str.data] == 0) &&
-                                   p > build_features) {
-                                       if (*(p-1) == '+') { // "+OPENSSL"
+                       ret = 0; // assume feature not found
+                       for (p = build_features; (p = strstr(p, term->args[0].data.str.area)); p++) {
+                               if (p > build_features &&
+                                   (p[term->args[0].data.str.data] == ' ' ||
+                                    p[term->args[0].data.str.data] == 0)) {
+                                       if (*(p-1) == '+') {       // e.g. "+OPENSSL"
                                                ret = 1;
-                                               goto done;
+                                               break;
                                        }
-                                       else if (*(p-1) == '-') { // "-OPENSSL"
+                                       else if (*(p-1) == '-') {  // e.g. "-OPENSSL"
                                                ret = 0;
-                                               goto done;
+                                               break;
                                        }
                                        /* it was a sub-word, let's restart from next place */
                                }
                        }
-                       /* not found */
-                       ret = 0;
-                       goto done;
+                       break;
                }
                case CFG_PRED_STREQ:    // checks if the two arg are equal
-                       ret = strcmp(argp[0].data.str.area, argp[1].data.str.area) == 0;
-                       goto done;
+                       ret = strcmp(term->args[0].data.str.area, term->args[1].data.str.area) == 0;
+                       break;
 
                case CFG_PRED_STRNEQ:   // checks if the two arg are different
-                       ret = strcmp(argp[0].data.str.area, argp[1].data.str.area) != 0;
-                       goto done;
+                       ret = strcmp(term->args[0].data.str.area, term->args[1].data.str.area) != 0;
+                       break;
 
                case CFG_PRED_VERSION_ATLEAST: // checks if the current version is at least this one
-                       ret = compare_current_version(argp[0].data.str.area) <= 0;
-                       goto done;
+                       ret = compare_current_version(term->args[0].data.str.area) <= 0;
+                       break;
 
                case CFG_PRED_VERSION_BEFORE:  // checks if the current version is older than this one
-                       ret = compare_current_version(argp[0].data.str.area) > 0;
-                       goto done;
+                       ret = compare_current_version(term->args[0].data.str.area) > 0;
+                       break;
 
                default:
-                       memprintf(err, "internal error: unhandled conditional expression predicate '%s'", cond_pred->word);
-                       if (errptr)
-                               *errptr = args[0];
-                       goto done;
+                       memprintf(err, "internal error: unhandled conditional expression predicate '%s'", term->pred->word);
+                       break;
                }
        }
+       else {
+               memprintf(err, "internal error: unhandled condition term type %d", (int)term->type);
+       }
+       return ret;
+}
+
+
+/* evaluate a condition on a .if/.elif line. The condition is already tokenized
+ * in <err>. Returns -1 on error (in which case err is filled with a message,
+ * and only in this case), 0 if the condition is false, 1 if it's true. If
+ * <errptr> is not NULL, it's set to the first invalid character on error.
+ */
+int cfg_eval_condition(char **args, char **err, const char **errptr)
+{
+       struct cfg_cond_term term = { };
+       const char *text = args[0];
+       int ret = -1;
+
+       if (!*text) /* note: empty = false */
+               return 0;
+
+       ret = cfg_parse_cond_term(&text, &term, err, errptr);
+       if (ret != 0) {
+               if (ret == -1) // parse error, error already reported
+                       goto done;
+               ret = cfg_eval_cond_term(&term, err);
+               goto done;
+       }
 
+       /* ret == 0, no other way to parse this */
+       ret = -1;
        memprintf(err, "unparsable conditional expression '%s'", args[0]);
        if (errptr)
-               *errptr = args[0];
+               *errptr = text;
  done:
-       free_args(argp);
-       ha_free(&argp);
+       free_args(term.args);
+       ha_free(&term.args);
        return ret;
 }