Skip to content

Commit 40f280c

Browse files
blpblp
committed
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 <shli@nicira.com>
1 parent 2c8fcc9 commit 40f280c

2 files changed

Lines changed: 87 additions & 58 deletions

File tree

‎ovsdb/condition.c‎

Lines changed: 51 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2009, 2010 Nicira Networks
1+
/* Copyright (c) 2009, 2010, 2011 Nicira Networks
22
*
33
* Licensed under the Apache License, Version 2.0 (the "License");
44
* you may not use this file except in compliance with the License.
@@ -217,55 +217,64 @@ ovsdb_condition_to_json(const struct ovsdb_condition *cnd)
217217
return json_array_create(clauses, cnd->n_clauses);
218218
}
219219

220+
static bool
221+
ovsdb_clause_evaluate(const struct ovsdb_row *row,
222+
const struct ovsdb_clause *c)
223+
{
224+
const struct ovsdb_datum *field = &row->fields[c->column->index];
225+
const struct ovsdb_datum *arg = &c->arg;
226+
const struct ovsdb_type *type = &c->column->type;
227+
228+
if (ovsdb_type_is_scalar(type)) {
229+
int cmp = ovsdb_atom_compare_3way(&field->keys[0], &arg->keys[0],
230+
type->key.type);
231+
switch (c->function) {
232+
case OVSDB_F_LT:
233+
return cmp < 0;
234+
case OVSDB_F_LE:
235+
return cmp <= 0;
236+
case OVSDB_F_EQ:
237+
case OVSDB_F_INCLUDES:
238+
return cmp == 0;
239+
case OVSDB_F_NE:
240+
case OVSDB_F_EXCLUDES:
241+
return cmp != 0;
242+
case OVSDB_F_GE:
243+
return cmp >= 0;
244+
case OVSDB_F_GT:
245+
return cmp > 0;
246+
}
247+
} else {
248+
switch (c->function) {
249+
case OVSDB_F_EQ:
250+
return ovsdb_datum_equals(field, arg, type);
251+
case OVSDB_F_NE:
252+
return !ovsdb_datum_equals(field, arg, type);
253+
case OVSDB_F_INCLUDES:
254+
return ovsdb_datum_includes_all(arg, field, type);
255+
case OVSDB_F_EXCLUDES:
256+
return ovsdb_datum_excludes_all(arg, field, type);
257+
case OVSDB_F_LT:
258+
case OVSDB_F_LE:
259+
case OVSDB_F_GE:
260+
case OVSDB_F_GT:
261+
NOT_REACHED();
262+
}
263+
}
264+
265+
NOT_REACHED();
266+
}
267+
220268
bool
221269
ovsdb_condition_evaluate(const struct ovsdb_row *row,
222270
const struct ovsdb_condition *cnd)
223271
{
224272
size_t i;
225273

226274
for (i = 0; i < cnd->n_clauses; i++) {
227-
const struct ovsdb_clause *c = &cnd->clauses[i];
228-
const struct ovsdb_datum *field = &row->fields[c->column->index];
229-
const struct ovsdb_datum *arg = &cnd->clauses[i].arg;
230-
const struct ovsdb_type *type = &c->column->type;
231-
232-
if (ovsdb_type_is_scalar(type)) {
233-
int cmp = ovsdb_atom_compare_3way(&field->keys[0], &arg->keys[0],
234-
type->key.type);
235-
switch (c->function) {
236-
case OVSDB_F_LT:
237-
return cmp < 0;
238-
case OVSDB_F_LE:
239-
return cmp <= 0;
240-
case OVSDB_F_EQ:
241-
case OVSDB_F_INCLUDES:
242-
return cmp == 0;
243-
case OVSDB_F_NE:
244-
case OVSDB_F_EXCLUDES:
245-
return cmp != 0;
246-
case OVSDB_F_GE:
247-
return cmp >= 0;
248-
case OVSDB_F_GT:
249-
return cmp > 0;
250-
}
251-
} else {
252-
switch (c->function) {
253-
case OVSDB_F_EQ:
254-
return ovsdb_datum_equals(field, arg, type);
255-
case OVSDB_F_NE:
256-
return !ovsdb_datum_equals(field, arg, type);
257-
case OVSDB_F_INCLUDES:
258-
return ovsdb_datum_includes_all(arg, field, type);
259-
case OVSDB_F_EXCLUDES:
260-
return ovsdb_datum_excludes_all(arg, field, type);
261-
case OVSDB_F_LT:
262-
case OVSDB_F_LE:
263-
case OVSDB_F_GE:
264-
case OVSDB_F_GT:
265-
NOT_REACHED();
266-
}
275+
if (!ovsdb_clause_evaluate(row, &cnd->clauses[i])) {
276+
return false;
267277
}
268-
NOT_REACHED();
269278
}
270279

271280
return true;

‎tests/ovsdb-condition.at‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on integers],
203203
[["i", ">=", 1]],
204204
[["i", ">", 1]],
205205
[["i", "includes", 1]],
206-
[["i", "excludes", 1]]]' \
206+
[["i", "excludes", 1]],
207+
[["i", ">", 0], ["i", "<", 2]]]' \
207208
'[{"i": 0},
208209
{"i": 1},
209210
{"i": 2}']]],
@@ -214,7 +215,8 @@ condition 3: T-T
214215
condition 4: -TT
215216
condition 5: --T
216217
condition 6: -T-
217-
condition 7: T-T], [condition])
218+
condition 7: T-T
219+
condition 8: -T-], [condition])
218220

219221
OVSDB_CHECK_POSITIVE([evaluating conditions on reals],
220222
[[evaluate-conditions \
@@ -226,7 +228,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on reals],
226228
[["r", ">=", 5.0]],
227229
[["r", ">", 5.0]],
228230
[["r", "includes", 5.0]],
229-
[["r", "excludes", 5.0]]]' \
231+
[["r", "excludes", 5.0]],
232+
[["r", "!=", 0], ["r", "!=", 5.1]]]' \
230233
'[{"r": 0},
231234
{"r": 5.0},
232235
{"r": 5.1}']]],
@@ -237,7 +240,8 @@ condition 3: T-T
237240
condition 4: -TT
238241
condition 5: --T
239242
condition 6: -T-
240-
condition 7: T-T], [condition])
243+
condition 7: T-T
244+
condition 8: -T-], [condition])
241245

242246
OVSDB_CHECK_POSITIVE([evaluating conditions on booleans],
243247
[[evaluate-conditions \
@@ -249,7 +253,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on booleans],
249253
[["b", "==", false]],
250254
[["b", "!=", false]],
251255
[["b", "includes", false]],
252-
[["b", "excludes", false]]]' \
256+
[["b", "excludes", false]],
257+
[["b", "==", true], ["b", "==", false]]]' \
253258
'[{"b": true},
254259
{"b": false}']]],
255260
[condition 0: T-
@@ -259,7 +264,8 @@ condition 3: -T
259264
condition 4: -T
260265
condition 5: T-
261266
condition 6: -T
262-
condition 7: T-], [condition])
267+
condition 7: T-
268+
condition 8: --], [condition])
263269

264270
OVSDB_CHECK_POSITIVE([evaluating conditions on strings],
265271
[[evaluate-conditions \
@@ -271,7 +277,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on strings],
271277
[["s", "==", "foo"]],
272278
[["s", "!=", "foo"]],
273279
[["s", "includes", "foo"]],
274-
[["s", "excludes", "foo"]]]' \
280+
[["s", "excludes", "foo"]],
281+
[["s", "!=", "foo"], ["s", "!=", ""]]]' \
275282
'[{"s": ""},
276283
{"s": "foo"},
277284
{"s": "xxx"}']]],
@@ -282,7 +289,8 @@ condition 3: -TT
282289
condition 4: -T-
283290
condition 5: T-T
284291
condition 6: -T-
285-
condition 7: T-T], [condition])
292+
condition 7: T-T
293+
condition 8: --T], [condition])
286294

287295
OVSDB_CHECK_POSITIVE([evaluating conditions on UUIDs],
288296
[[evaluate-conditions \
@@ -294,7 +302,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on UUIDs],
294302
[["u", "==", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]],
295303
[["u", "!=", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]],
296304
[["u", "includes", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]],
297-
[["u", "excludes", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]]]' \
305+
[["u", "excludes", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]],
306+
[["u", "!=", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]],
307+
["u", "!=", ["uuid", "cb160ed6-92a6-4503-a6aa-a09a09e01f0d"]]]]' \
298308
'[{"u": ["uuid", "8a1dbdb8-416f-4ce9-affa-3332691714b6"]},
299309
{"u": ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]},
300310
{"u": ["uuid", "00000000-0000-0000-0000-000000000000"]}']]],
@@ -305,7 +315,8 @@ condition 3: -TT
305315
condition 4: -T-
306316
condition 5: T-T
307317
condition 6: -T-
308-
condition 7: T-T], [condition])
318+
condition 7: T-T
319+
condition 8: T-T], [condition])
309320

310321
OVSDB_CHECK_POSITIVE([evaluating conditions on sets],
311322
[[evaluate-conditions \
@@ -341,7 +352,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on sets],
341352
[["i", "excludes", ["set", [2]]]],
342353
[["i", "excludes", ["set", [2, 0]]]],
343354
[["i", "excludes", ["set", [2, 1]]]],
344-
[["i", "excludes", ["set", [2, 1, 0]]]]]' \
355+
[["i", "excludes", ["set", [2, 1, 0]]]],
356+
[["i", "includes", ["set", [0]]],
357+
["i", "includes", ["set", [1]]]]]' \
345358
'[{"i": ["set", []]},
346359
{"i": ["set", [0]]},
347360
{"i": ["set", [1]]},
@@ -382,7 +395,8 @@ condition 27: T---T ---
382395
condition 28: TTTT- ---
383396
condition 29: T-T-- ---
384397
condition 30: TT--- ---
385-
condition 31: T---- ---], [condition])
398+
condition 31: T---- ---
399+
condition 32: ---T- --T], [condition])
386400

387401
# This is the same as the "set" test except that it adds values,
388402
# all of which always match.
@@ -423,7 +437,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on maps (1)],
423437
[["i", "excludes", ["map", [[2, true]]]]],
424438
[["i", "excludes", ["map", [[2, true], [0, true]]]]],
425439
[["i", "excludes", ["map", [[2, true], [1, false]]]]],
426-
[["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]]]' \
440+
[["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]],
441+
[["i", "includes", ["map", [[0, true]]]],
442+
["i", "includes", ["map", [[1, false]]]]]]' \
427443
'[{"i": ["map", []]},
428444
{"i": ["map", [[0, true]]]},
429445
{"i": ["map", [[1, false]]]},
@@ -464,7 +480,8 @@ condition 27: T---T ---
464480
condition 28: TTTT- ---
465481
condition 29: T-T-- ---
466482
condition 30: TT--- ---
467-
condition 31: T---- ---], [condition])
483+
condition 31: T---- ---
484+
condition 32: ---T- --T], [condition])
468485

469486
# This is the same as the "set" test except that it adds values,
470487
# and those values don't always match.
@@ -505,7 +522,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on maps (2)],
505522
[["i", "excludes", ["map", [[2, true]]]]],
506523
[["i", "excludes", ["map", [[2, true], [0, true]]]]],
507524
[["i", "excludes", ["map", [[2, true], [1, false]]]]],
508-
[["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]]]' \
525+
[["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]],
526+
[["i", "includes", ["map", [[0, true]]]],
527+
["i", "includes", ["map", [[1, false]]]]]]' \
509528
'[{"i": ["map", []]},
510529
{"i": ["map", [[0, true]]]},
511530
{"i": ["map", [[0, false]]]},
@@ -555,4 +574,5 @@ condition 27: T-T-T --TT- --T--
555574
condition 28: TTTTT TT-T- T----
556575
condition 29: T-TTT ---T- -----
557576
condition 30: TTT-T -T-T- T----
558-
condition 31: T-T-T ---T- -----], [condition])
577+
condition 31: T-T-T ---T- -----
578+
condition 32: ----- T---- ---T-], [condition])

0 commit comments

Comments
 (0)