[ovs-dev] [PATCH 2/3] expr: Make expr_sort() always yield an expr that satisfies invariants.
Numan Siddique
nusiddiq at redhat.com
Wed Feb 7 12:57:45 UTC 2018
On Tue, Feb 6, 2018 at 11:30 PM, Ben Pfaff <blp at ovn.org> wrote:
> 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 <blp at ovn.org>
>
Acked-by: Numan Siddique <nusiddiq at redhat.com>
Thanks
Numan
> ---
> 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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list