From 40f280c781639c24176e054abc20bb7270da9e7a Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 16 Nov 2011 14:38:52 -0800 Subject: [PATCH] ovsdb: Correctly implement conditions that include multiple clauses. Multiple-clause conditions in OVSDB operations with "where" clauses are supposed to be conjunctions, that is, the condition is true only if every clause is true. In fact, the implementation only checked a single clause (not necessarily the first one) and ignored the rest. This fixes the problem and adds test coverage for multiple-clause conditions. Reported-by: Shih-Hao Li --- ovsdb/condition.c | 93 ++++++++++++++++++++++------------------ tests/ovsdb-condition.at | 52 +++++++++++++++------- 2 files changed, 87 insertions(+), 58 deletions(-) diff --git a/ovsdb/condition.c b/ovsdb/condition.c index 59f742c9..c57e4197 100644 --- a/ovsdb/condition.c +++ b/ovsdb/condition.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010 Nicira Networks +/* Copyright (c) 2009, 2010, 2011 Nicira Networks * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -217,6 +217,54 @@ ovsdb_condition_to_json(const struct ovsdb_condition *cnd) return json_array_create(clauses, cnd->n_clauses); } +static bool +ovsdb_clause_evaluate(const struct ovsdb_row *row, + const struct ovsdb_clause *c) +{ + const struct ovsdb_datum *field = &row->fields[c->column->index]; + const struct ovsdb_datum *arg = &c->arg; + const struct ovsdb_type *type = &c->column->type; + + if (ovsdb_type_is_scalar(type)) { + int cmp = ovsdb_atom_compare_3way(&field->keys[0], &arg->keys[0], + type->key.type); + switch (c->function) { + case OVSDB_F_LT: + return cmp < 0; + case OVSDB_F_LE: + return cmp <= 0; + case OVSDB_F_EQ: + case OVSDB_F_INCLUDES: + return cmp == 0; + case OVSDB_F_NE: + case OVSDB_F_EXCLUDES: + return cmp != 0; + case OVSDB_F_GE: + return cmp >= 0; + case OVSDB_F_GT: + return cmp > 0; + } + } else { + switch (c->function) { + case OVSDB_F_EQ: + return ovsdb_datum_equals(field, arg, type); + case OVSDB_F_NE: + return !ovsdb_datum_equals(field, arg, type); + case OVSDB_F_INCLUDES: + return ovsdb_datum_includes_all(arg, field, type); + case OVSDB_F_EXCLUDES: + return ovsdb_datum_excludes_all(arg, field, type); + case OVSDB_F_LT: + case OVSDB_F_LE: + case OVSDB_F_GE: + case OVSDB_F_GT: + NOT_REACHED(); + } + } + + NOT_REACHED(); +} + bool ovsdb_condition_evaluate(const struct ovsdb_row *row, const struct ovsdb_condition *cnd) @@ -224,48 +272,9 @@ ovsdb_condition_evaluate(const struct ovsdb_row *row, size_t i; for (i = 0; i < cnd->n_clauses; i++) { - const struct ovsdb_clause *c = &cnd->clauses[i]; - const struct ovsdb_datum *field = &row->fields[c->column->index]; - const struct ovsdb_datum *arg = &cnd->clauses[i].arg; - const struct ovsdb_type *type = &c->column->type; - - if (ovsdb_type_is_scalar(type)) { - int cmp = ovsdb_atom_compare_3way(&field->keys[0], &arg->keys[0], - type->key.type); - switch (c->function) { - case OVSDB_F_LT: - return cmp < 0; - case OVSDB_F_LE: - return cmp <= 0; - case OVSDB_F_EQ: - case OVSDB_F_INCLUDES: - return cmp == 0; - case OVSDB_F_NE: - case OVSDB_F_EXCLUDES: - return cmp != 0; - case OVSDB_F_GE: - return cmp >= 0; - case OVSDB_F_GT: - return cmp > 0; - } - } else { - switch (c->function) { - case OVSDB_F_EQ: - return ovsdb_datum_equals(field, arg, type); - case OVSDB_F_NE: - return !ovsdb_datum_equals(field, arg, type); - case OVSDB_F_INCLUDES: - return ovsdb_datum_includes_all(arg, field, type); - case OVSDB_F_EXCLUDES: - return ovsdb_datum_excludes_all(arg, field, type); - case OVSDB_F_LT: - case OVSDB_F_LE: - case OVSDB_F_GE: - case OVSDB_F_GT: - NOT_REACHED(); - } + if (!ovsdb_clause_evaluate(row, &cnd->clauses[i])) { + return false; } - NOT_REACHED(); } return true; diff --git a/tests/ovsdb-condition.at b/tests/ovsdb-condition.at index 80961cc5..d3d7d83c 100644 --- a/tests/ovsdb-condition.at +++ b/tests/ovsdb-condition.at @@ -203,7 +203,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on integers], [["i", ">=", 1]], [["i", ">", 1]], [["i", "includes", 1]], - [["i", "excludes", 1]]]' \ + [["i", "excludes", 1]], + [["i", ">", 0], ["i", "<", 2]]]' \ '[{"i": 0}, {"i": 1}, {"i": 2}']]], @@ -214,7 +215,8 @@ condition 3: T-T condition 4: -TT condition 5: --T condition 6: -T- -condition 7: T-T], [condition]) +condition 7: T-T +condition 8: -T-], [condition]) OVSDB_CHECK_POSITIVE([evaluating conditions on reals], [[evaluate-conditions \ @@ -226,7 +228,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on reals], [["r", ">=", 5.0]], [["r", ">", 5.0]], [["r", "includes", 5.0]], - [["r", "excludes", 5.0]]]' \ + [["r", "excludes", 5.0]], + [["r", "!=", 0], ["r", "!=", 5.1]]]' \ '[{"r": 0}, {"r": 5.0}, {"r": 5.1}']]], @@ -237,7 +240,8 @@ condition 3: T-T condition 4: -TT condition 5: --T condition 6: -T- -condition 7: T-T], [condition]) +condition 7: T-T +condition 8: -T-], [condition]) OVSDB_CHECK_POSITIVE([evaluating conditions on booleans], [[evaluate-conditions \ @@ -249,7 +253,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on booleans], [["b", "==", false]], [["b", "!=", false]], [["b", "includes", false]], - [["b", "excludes", false]]]' \ + [["b", "excludes", false]], + [["b", "==", true], ["b", "==", false]]]' \ '[{"b": true}, {"b": false}']]], [condition 0: T- @@ -259,7 +264,8 @@ condition 3: -T condition 4: -T condition 5: T- condition 6: -T -condition 7: T-], [condition]) +condition 7: T- +condition 8: --], [condition]) OVSDB_CHECK_POSITIVE([evaluating conditions on strings], [[evaluate-conditions \ @@ -271,7 +277,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on strings], [["s", "==", "foo"]], [["s", "!=", "foo"]], [["s", "includes", "foo"]], - [["s", "excludes", "foo"]]]' \ + [["s", "excludes", "foo"]], + [["s", "!=", "foo"], ["s", "!=", ""]]]' \ '[{"s": ""}, {"s": "foo"}, {"s": "xxx"}']]], @@ -282,7 +289,8 @@ condition 3: -TT condition 4: -T- condition 5: T-T condition 6: -T- -condition 7: T-T], [condition]) +condition 7: T-T +condition 8: --T], [condition]) OVSDB_CHECK_POSITIVE([evaluating conditions on UUIDs], [[evaluate-conditions \ @@ -294,7 +302,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on UUIDs], [["u", "==", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]], [["u", "!=", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]], [["u", "includes", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]], - [["u", "excludes", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]]]' \ + [["u", "excludes", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]], + [["u", "!=", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]], + ["u", "!=", ["uuid", "cb160ed6-92a6-4503-a6aa-a09a09e01f0d"]]]]' \ '[{"u": ["uuid", "8a1dbdb8-416f-4ce9-affa-3332691714b6"]}, {"u": ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]}, {"u": ["uuid", "00000000-0000-0000-0000-000000000000"]}']]], @@ -305,7 +315,8 @@ condition 3: -TT condition 4: -T- condition 5: T-T condition 6: -T- -condition 7: T-T], [condition]) +condition 7: T-T +condition 8: T-T], [condition]) OVSDB_CHECK_POSITIVE([evaluating conditions on sets], [[evaluate-conditions \ @@ -341,7 +352,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on sets], [["i", "excludes", ["set", [2]]]], [["i", "excludes", ["set", [2, 0]]]], [["i", "excludes", ["set", [2, 1]]]], - [["i", "excludes", ["set", [2, 1, 0]]]]]' \ + [["i", "excludes", ["set", [2, 1, 0]]]], + [["i", "includes", ["set", [0]]], + ["i", "includes", ["set", [1]]]]]' \ '[{"i": ["set", []]}, {"i": ["set", [0]]}, {"i": ["set", [1]]}, @@ -382,7 +395,8 @@ condition 27: T---T --- condition 28: TTTT- --- condition 29: T-T-- --- condition 30: TT--- --- -condition 31: T---- ---], [condition]) +condition 31: T---- --- +condition 32: ---T- --T], [condition]) # This is the same as the "set" test except that it adds values, # all of which always match. @@ -423,7 +437,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on maps (1)], [["i", "excludes", ["map", [[2, true]]]]], [["i", "excludes", ["map", [[2, true], [0, true]]]]], [["i", "excludes", ["map", [[2, true], [1, false]]]]], - [["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]]]' \ + [["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]], + [["i", "includes", ["map", [[0, true]]]], + ["i", "includes", ["map", [[1, false]]]]]]' \ '[{"i": ["map", []]}, {"i": ["map", [[0, true]]]}, {"i": ["map", [[1, false]]]}, @@ -464,7 +480,8 @@ condition 27: T---T --- condition 28: TTTT- --- condition 29: T-T-- --- condition 30: TT--- --- -condition 31: T---- ---], [condition]) +condition 31: T---- --- +condition 32: ---T- --T], [condition]) # This is the same as the "set" test except that it adds values, # and those values don't always match. @@ -505,7 +522,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on maps (2)], [["i", "excludes", ["map", [[2, true]]]]], [["i", "excludes", ["map", [[2, true], [0, true]]]]], [["i", "excludes", ["map", [[2, true], [1, false]]]]], - [["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]]]' \ + [["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]], + [["i", "includes", ["map", [[0, true]]]], + ["i", "includes", ["map", [[1, false]]]]]]' \ '[{"i": ["map", []]}, {"i": ["map", [[0, true]]]}, {"i": ["map", [[0, false]]]}, @@ -555,4 +574,5 @@ condition 27: T-T-T --TT- --T-- condition 28: TTTTT TT-T- T---- condition 29: T-TTT ---T- ----- condition 30: TTT-T -T-T- T---- -condition 31: T-T-T ---T- -----], [condition]) +condition 31: T-T-T ---T- ----- +condition 32: ----- T---- ---T-], [condition]) -- 2.30.2