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 <[email protected]>
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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
