[ovs-dev] [PATCH 1/3] expr: Fix some bad naming.
Numan Siddique
nusiddiq at redhat.com
Wed Feb 7 12:57:27 UTC 2018
On Tue, Feb 6, 2018 at 11:30 PM, Ben Pfaff <blp at ovn.org> wrote:
> expr_is_cmp() was badly named because it didn't just check for whether its
> argument was an EXPR_T_CMP node.
>
> struct expr_sort's 'relop' member was badly named because it wasn't a
> relational operator, it was a symbol.
>
> This commit improves both names.
>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
>
Acked-by: Numan Siddique <nusiddiq at redhat.com>
Thanks
Numan
> ---
> ovn/lib/expr.c | 46 ++++++++++++++++++++++++++--------------------
> 1 file changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
> index 79ff45762f65..108af4a48210 100644
> --- a/ovn/lib/expr.c
> +++ b/ovn/lib/expr.c
> @@ -1922,8 +1922,13 @@ expr_simplify(struct expr *expr,
> OVS_NOT_REACHED();
> }
>
> +/* Tests whether 'expr' is an expression over exactly one symbol: that is,
> + * whether it is either a EXPR_T_CMP node or a tree of ANDs and ORs all
> over
> + * the same symbol. If it is, returns the symbol in question. If it is
> not
> + * (that is, if there is more than one symbol or no symbols at all),
> returns
> + * NULL. */
> static const struct expr_symbol *
> -expr_is_cmp(const struct expr *expr)
> +expr_get_unique_symbol(const struct expr *expr)
> {
> switch (expr->type) {
> case EXPR_T_CMP:
> @@ -1935,7 +1940,7 @@ expr_is_cmp(const struct expr *expr)
> struct expr *sub;
>
> LIST_FOR_EACH (sub, node, &expr->andor) {
> - const struct expr_symbol *symbol = expr_is_cmp(sub);
> + const struct expr_symbol *symbol =
> expr_get_unique_symbol(sub);
> if (!symbol || (prev && symbol != prev)) {
> return NULL;
> }
> @@ -1955,7 +1960,7 @@ expr_is_cmp(const struct expr *expr)
>
> struct expr_sort {
> struct expr *expr;
> - const struct expr_symbol *relop;
> + const struct expr_symbol *symbol;
> enum expr_type type;
> };
>
> @@ -1967,8 +1972,8 @@ compare_expr_sort(const void *a_, const void *b_)
>
> if (a->type != b->type) {
> return a->type < b->type ? -1 : 1;
> - } else if (a->relop) {
> - int cmp = strcmp(a->relop->name, b->relop->name);
> + } else if (a->symbol) {
> + int cmp = strcmp(a->symbol->name, b->symbol->name);
> if (cmp) {
> return cmp;
> }
> @@ -2330,10 +2335,10 @@ crush_or(struct expr *expr, const struct
> expr_symbol *symbol)
> return expr_fix(expr);
> }
>
> -/* Takes ownership of 'expr', which must be a cmp in the sense determined
> by
> - * 'expr_is_cmp(expr)', where 'symbol' is the symbol returned by that
> function.
> - * Returns an equivalent expression owned by the caller that is a single
> - * EXPR_T_CMP or a disjunction of them or a EXPR_T_BOOLEAN. */
> +/* Takes ownership of 'expr', which must have a unique symbol in the
> sense of
> + * 'expr_get_unique_symbol(expr)', where 'symbol' is the symbol returned
> by
> + * that function. Returns an equivalent expression owned by the caller
> that is
> + * a single EXPR_T_CMP or a disjunction of them or a EXPR_T_BOOLEAN. */
> static struct expr *
> crush_cmps(struct expr *expr, const struct expr_symbol *symbol)
> {
> @@ -2372,8 +2377,8 @@ expr_sort(struct expr *expr)
> i = 0;
> LIST_FOR_EACH (sub, node, &expr->andor) {
> subs[i].expr = sub;
> - subs[i].relop = expr_is_cmp(sub);
> - subs[i].type = subs[i].relop ? EXPR_T_CMP : sub->type;
> + subs[i].symbol = expr_get_unique_symbol(sub);
> + subs[i].type = subs[i].symbol ? EXPR_T_CMP : sub->type;
> i++;
> }
> ovs_assert(i == n);
> @@ -2382,17 +2387,17 @@ expr_sort(struct expr *expr)
>
> ovs_list_init(&expr->andor);
> for (i = 0; i < n; ) {
> - if (subs[i].relop) {
> + if (subs[i].symbol) {
> size_t j;
> for (j = i + 1; j < n; j++) {
> - if (subs[i].relop != subs[j].relop) {
> + if (subs[i].symbol != subs[j].symbol) {
> break;
> }
> }
>
> struct expr *crushed;
> if (j == i + 1) {
> - crushed = crush_cmps(subs[i].expr, subs[i].relop);
> + crushed = crush_cmps(subs[i].expr, subs[i].symbol);
> } else {
> struct expr *combined = subs[i].expr;
> for (size_t k = i + 1; k < j; k++) {
> @@ -2400,7 +2405,7 @@ expr_sort(struct expr *expr)
> subs[k].expr);
> }
> ovs_assert(!ovs_list_is_short(&combined->andor));
> - crushed = crush_cmps(combined, subs[i].relop);
> + crushed = crush_cmps(combined, subs[i].symbol);
> }
> if (crushed->type == EXPR_T_BOOLEAN) {
> if (!crushed->boolean) {
> @@ -2472,7 +2477,7 @@ expr_normalize_and(struct expr *expr)
> }
>
> ovs_assert(sub->type == EXPR_T_OR);
> - const struct expr_symbol *symbol = expr_is_cmp(sub);
> + const struct expr_symbol *symbol = expr_get_unique_symbol(sub);
> if (!symbol || symbol->must_crossproduct) {
> struct expr *or = expr_create_andor(EXPR_T_OR);
> struct expr *k;
> @@ -2842,7 +2847,7 @@ expr_to_matches(const struct expr *expr,
> break;
>
> case EXPR_T_OR:
> - if (expr_is_cmp(expr)) {
> + if (expr_get_unique_symbol(expr)) {
> struct expr *sub;
>
> LIST_FOR_EACH (sub, node, &expr->andor) {
> @@ -2966,7 +2971,7 @@ expr_is_normalized_and(const struct expr *expr)
> const struct expr *sub;
>
> LIST_FOR_EACH (sub, node, &expr->andor) {
> - if (!expr_is_cmp(sub)) {
> + if (!expr_get_unique_symbol(sub)) {
> return false;
> }
> }
> @@ -2986,11 +2991,12 @@ expr_is_normalized(const struct expr *expr)
> return expr_is_normalized_and(expr);
>
> case EXPR_T_OR:
> - if (!expr_is_cmp(expr)) {
> + if (!expr_get_unique_symbol(expr)) {
> const struct expr *sub;
>
> LIST_FOR_EACH (sub, node, &expr->andor) {
> - if (!expr_is_cmp(sub) && !expr_is_normalized_and(sub)) {
> + if (!expr_get_unique_symbol(sub)
> + && !expr_is_normalized_and(sub)) {
> return false;
> }
> }
> --
> 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