Expressions of type EXPR_T_AND are supposed to follow an invariant that they have at least 2 clauses, but expr_sort() did not always follow that; for example, applying it to (x[0] == 1 && x[1] == 1) yielded the 1-child EXPR_T_AND expression x[0..1] == 3. This commit fixes the problem.
I don't know of any externally visible negative consequences for this problem, but it made the code harder to reason about. Signed-off-by: Ben Pfaff <[email protected]> --- ovn/lib/expr.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c index 108af4a48210..b0aa6726b5e0 100644 --- a/ovn/lib/expr.c +++ b/ovn/lib/expr.c @@ -2366,9 +2366,19 @@ crush_cmps(struct expr *expr, const struct expr_symbol *symbol) } } +/* Applied to an EXPR_T_AND 'expr' whose subexpressions are in terms of only + * EXPR_T_CMP, EXPR_T_AND, and EXPR_T_OR, this takes ownership of 'expr' and + * returns a new expression in terms of EXPR_T_CMP, EXPR_T_AND, EXPR_T_OR, or + * EXPR_T_BOOLEAN. + * + * The function attempts to bring together and combine clauses of the original + * 'expr' that were in terms of a single variable. For example, it combines + * (x[0] == 1 && x[1] == 1) into the single x[0..1] == 3. */ static struct expr * expr_sort(struct expr *expr) { + ovs_assert(expr->type == EXPR_T_AND); + size_t n = ovs_list_size(&expr->andor); struct expr_sort *subs = xmalloc(n * sizeof *subs); struct expr *sub; @@ -2386,6 +2396,9 @@ expr_sort(struct expr *expr) qsort(subs, n, sizeof *subs, compare_expr_sort); ovs_list_init(&expr->andor); + free(expr); + expr = NULL; + for (i = 0; i < n; ) { if (subs[i].symbol) { size_t j; @@ -2438,11 +2451,8 @@ static struct expr *expr_normalize_or(struct expr *expr); static struct expr * expr_normalize_and(struct expr *expr) { - ovs_assert(expr->type == EXPR_T_AND); - expr = expr_sort(expr); if (expr->type != EXPR_T_AND) { - ovs_assert(expr->type == EXPR_T_BOOLEAN); return expr; } -- 2.15.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
