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

Reply via email to