On Tue, May 17, 2016 at 05:27:00PM +0300, Liran Schour wrote: > Change ovsdb_condition to be a 3-element json array or a boolean value. > Conditions utilities will be used later for conditional monitoring. > > Signed-off-by: Liran Schour <lir...@il.ibm.com>
I believe that this actually extends the OVSDB protocol. When we make extensions, we document them in ovsdb/ovsdb-server.1.in, in the section titled SPECIFICATIONS, so please add a description there. The code in ovsdb_clause_from_json() for converting JSON_TRUE into OVSDB_F_TRUE and JSON_FALSE into OVSDB_F_FALSE seems awfully indirect. Why not just use the enums directly? + if (json->type == JSON_TRUE || json->type == JSON_FALSE) { + function_name = (json->type == JSON_TRUE) ? "true" : "false"; + error = ovsdb_function_from_string(function_name, &clause->function); Please use capital letters and punctuation in comments: + /* column and arg fields are not being used with boolean function + * use dummy values */ It should not be possible to hit these cases in ovsdb_clause_from_json(), so please use OVS_NOT_REACHED(): + case OVSDB_F_TRUE: + case OVSDB_F_FALSE: There are a couple of instances of <boolean-expression> ? true : false in this patch. Please just drop the ?: part, it's a no-op, e.g. + return json_boolean_create(clause->function == OVSDB_F_TRUE ? + true : false); becomes + return json_boolean_create(clause->function == OVSDB_F_TRUE); Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev