[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